[PATCH 0/3] ASoC: Intel: Skylake: Some more fixes

[PATCH 0/3] ASoC: Intel: Skylake: Some more fixes

[Vinod Koul]

This would hopefully be last round of fixes required to have SKL
completely functional on upstream.

First patch is actually only one required for that which adds
missing machine entry for SKL boards with RT286.  Rest of the two
are fixes for static checker warnings reported by folks.

Jeeja KP (1):
  ASoC: Intel: Skylake: fix missing machine device creation

Subhransu S. Prusty (1):
  ASoC: Intel: Skylake: Fix to cleanup if skl_sst_dsp_init fails

Vinod Koul (1):
  ASoC: Intel: Skylake: Fix substream dereference before check

 sound/soc/intel/skylake/skl-pcm.c |  3 +-
 sound/soc/intel/skylake/skl-sst.c |  7 ++--
 sound/soc/intel/skylake/skl.c     | 79 ++++++++++++++++++++++++++++++++++++++-
 sound/soc/intel/skylake/skl.h     |  1 +
 4 files changed, 84 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Vinod Koul]

From: Jeeja KP <jeeja.kp@intel.com>

The UEFI BIOS does not create a machine entry for Linux devices
so add a table style machine registration to fix this missing
entry

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Omair M Abdullah <omair.m.abdullah@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl.c | 79 +++++++++++++++++++++++++++++++++++++++++--
 sound/soc/intel/skylake/skl.h |  1 +
 2 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 9b94a8cdf9bd..e399a6734cf9 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -28,6 +28,11 @@
 #include <sound/pcm.h>
 #include "skl.h"
 
+struct sst_machines {
+	char *codec_id;
+	char *machine;
+};
+
 /*
  * initialize the PCI registers
  */
@@ -251,6 +256,64 @@ static int skl_free(struct hdac_ext_bus *ebus)
 	return 0;
 }
 
+static acpi_status sst_acpi_mach_match(acpi_handle handle, u32 level,
+				       void *context, void **ret)
+{
+	*(bool *)context = true;
+	return AE_OK;
+}
+
+static struct sst_machines *sst_acpi_find_machine(
+	struct sst_machines *machines)
+{
+	struct sst_machines *mach;
+	bool found = false;
+
+	for (mach = machines; mach->codec_id; mach++)
+		if (ACPI_SUCCESS(acpi_get_devices(mach->codec_id,
+						  sst_acpi_mach_match,
+						  &found, NULL)) && found)
+			return mach;
+
+	return NULL;
+}
+
+static int skl_machine_device_register(struct skl *skl, void *driver_data)
+{
+	struct hdac_bus *bus = ebus_to_hbus(&skl->ebus);
+	struct platform_device *pdev;
+	struct sst_machines *mach = driver_data;
+	int ret;
+
+	mach = sst_acpi_find_machine(mach);
+	if (mach == NULL) {
+		dev_err(bus->dev, "No matching machine driver found\n");
+		return -ENODEV;
+	}
+
+	pdev = platform_device_alloc(mach->machine, -1);
+	if (pdev == NULL) {
+		dev_err(bus->dev, "platform device alloc failed\n");
+		return -EIO;
+	}
+
+	ret = platform_device_add(pdev);
+	if (ret) {
+		dev_err(bus->dev, "failed to add machine device\n");
+		platform_device_put(pdev);
+		return -EIO;
+	}
+	skl->i2s_dev = pdev;
+
+	return 0;
+}
+
+static void skl_machine_device_unregister(struct skl *skl)
+{
+	if (skl->i2s_dev)
+		platform_device_unregister(skl->i2s_dev);
+}
+
 static int skl_dmic_device_register(struct skl *skl)
 {
 	struct hdac_bus *bus = ebus_to_hbus(&skl->ebus);
@@ -484,6 +547,10 @@ static int skl_probe(struct pci_dev *pci,
 			dev_dbg(bus->dev, "error failed to register dsp\n");
 			goto out_free;
 		}
+		err = skl_machine_device_register(skl,
+						  (void *)pci_id->driver_data);
+		if (err < 0)
+			goto out_dsp_free;
 	}
 	if (ebus->mlcap)
 		snd_hdac_ext_bus_get_ml_capabilities(ebus);
@@ -491,7 +558,7 @@ static int skl_probe(struct pci_dev *pci,
 	/* create device for soc dmic */
 	err = skl_dmic_device_register(skl);
 	if (err < 0)
-		goto out_dsp_free;
+		goto out_mach_free;
 
 	/* register platform dai and controls */
 	err = skl_platform_register(bus->dev);
@@ -515,6 +582,8 @@ out_unregister:
 	skl_platform_unregister(bus->dev);
 out_dmic_free:
 	skl_dmic_device_unregister(skl);
+out_mach_free:
+	skl_machine_device_unregister(skl);
 out_dsp_free:
 	skl_free_dsp(skl);
 out_free:
@@ -534,15 +603,21 @@ static void skl_remove(struct pci_dev *pci)
 	pci_dev_put(pci);
 	skl_platform_unregister(&pci->dev);
 	skl_free_dsp(skl);
+	skl_machine_device_unregister(skl);
 	skl_dmic_device_unregister(skl);
 	skl_free(ebus);
 	dev_set_drvdata(&pci->dev, NULL);
 }
 
+static struct sst_machines sst_skl_devdata[] = {
+	{ "INT343A", "skl_alc286s_i2s" },
+};
+
 /* PCI IDs */
 static const struct pci_device_id skl_ids[] = {
 	/* Sunrise Point-LP */
-	{ PCI_DEVICE(0x8086, 0x9d70), 0},
+	{ PCI_DEVICE(0x8086, 0x9d70),
+		.driver_data = (unsigned long)&sst_skl_devdata},
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, skl_ids);
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index f803ebb10605..9b1beed26f0f 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -61,6 +61,7 @@ struct skl {
 
 	unsigned int init_failed:1; /* delayed init failed */
 	struct platform_device *dmic_dev;
+	struct platform_device *i2s_dev;
 
 	void *nhlt; /* nhlt ptr */
 	struct skl_sst *skl_sst; /* sst skl ctx */
-- 
1.9.1

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Mark Brown]

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

On Fri, Oct 30, 2015 at 08:34:18PM +0530, Vinod Koul wrote:
> From: Jeeja KP <jeeja.kp@intel.com>
> 
> The UEFI BIOS does not create a machine entry for Linux devices
> so add a table style machine registration to fix this missing
> entry

How does this relate to the existing sst_acpi code?  This might be
clearer with a user...

> +static struct sst_machines *sst_acpi_find_machine(
> +	struct sst_machines *machines)
> +{
> +	struct sst_machines *mach;
> +	bool found = false;
> +
> +	for (mach = machines; mach->codec_id; mach++)
> +		if (ACPI_SUCCESS(acpi_get_devices(mach->codec_id,
> +						  sst_acpi_mach_match,
> +						  &found, NULL)) && found)
> +			return mach;
> +
> +	return NULL;
> +}

...but the code looks extremely similar.

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

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

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Vinod Koul]

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

On Sun, Nov 01, 2015 at 11:54:38AM +0900, Mark Brown wrote:
> On Fri, Oct 30, 2015 at 08:34:18PM +0530, Vinod Koul wrote:
> > From: Jeeja KP <jeeja.kp@intel.com>
> > 
> > The UEFI BIOS does not create a machine entry for Linux devices
> > so add a table style machine registration to fix this missing
> > entry
> 
> How does this relate to the existing sst_acpi code?  This might be
> clearer with a user...

It is similar but sst_machines structure is entirely different. In Skylake
case we only need machine entry name and rest of the information is coming
from topology manifest data. I will try to move this as well to topology
data later, but for now on upstream to get audio out from boards I need this
patch :)

I am not sure I follow the comment on user, the SKL driver here is user in
this

> > +static struct sst_machines *sst_acpi_find_machine(
> > +	struct sst_machines *machines)
> > +{
> > +	struct sst_machines *mach;
> > +	bool found = false;
> > +
> > +	for (mach = machines; mach->codec_id; mach++)
> > +		if (ACPI_SUCCESS(acpi_get_devices(mach->codec_id,
> > +						  sst_acpi_mach_match,
> > +						  &found, NULL)) && found)
> > +			return mach;
> > +
> > +	return NULL;
> > +}
> 
> ...but the code looks extremely similar.

as explained above code is similar but everyone uses different sst_machines
strcuture.

-- 
~Vinod

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Mark Brown]

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

On Mon, Nov 02, 2015 at 03:41:08PM +0530, Vinod Koul wrote:
> On Sun, Nov 01, 2015 at 11:54:38AM +0900, Mark Brown wrote:

> > > The UEFI BIOS does not create a machine entry for Linux devices
> > > so add a table style machine registration to fix this missing
> > > entry

> > How does this relate to the existing sst_acpi code?  This might be
> > clearer with a user...

> It is similar but sst_machines structure is entirely different. In Skylake
> case we only need machine entry name and rest of the information is coming

So what exactly is the "machine entry name" supposed to be here and why
don't we use any board specific information?

> from topology manifest data. I will try to move this as well to topology
> data later, but for now on upstream to get audio out from boards I need this
> patch :)

I'm not sure I entirely believe that this is going to work well in
practice TBH.

> I am not sure I follow the comment on user, the SKL driver here is user in
> this

There are no machines defined for this.

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

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

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Vinod Koul]

On Mon, Nov 02, 2015 at 12:07:27PM +0000, Mark Brown wrote:
> On Mon, Nov 02, 2015 at 03:41:08PM +0530, Vinod Koul wrote:
> > On Sun, Nov 01, 2015 at 11:54:38AM +0900, Mark Brown wrote:
> 
> > > > The UEFI BIOS does not create a machine entry for Linux devices
> > > > so add a table style machine registration to fix this missing
> > > > entry
> 
> > > How does this relate to the existing sst_acpi code?  This might be
> > > clearer with a user...
> 
> > It is similar but sst_machines structure is entirely different. In Skylake
> > case we only need machine entry name and rest of the information is coming
> 
> So what exactly is the "machine entry name" supposed to be here and why
> don't we use any board specific information?

For this case the machine entry name is "skl_alc286s_i2s", which is the RT
ALC 286 codec combination on Skylake.
The SKL machine driver needs this as platform device and we create it here

> +static struct sst_machines sst_skl_devdata[] = {
> +	{ "INT343A", "skl_alc286s_i2s" },
> +};

This says for SKL, with codec ID "INT343A" create "skl_alc286s_i2s" machine
platform device
For different codec combination we can use codec ACPI names to match

> I'm not sure I entirely believe that this is going to work well in
> practice TBH.

This is same way used for other Intel platfroms, but agreed this is not
elegant and I am looking at removing this with the help of topology, but not
there yet so need this one for a while :)

> > I am not sure I follow the comment on user, the SKL driver here is user in
> > this
> 
> There are no machines defined for this.

sound/soc/intel/boards/skl_rt286.c

-- 
~Vinod

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Mark Brown]

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

On Mon, Nov 02, 2015 at 09:02:29PM +0530, Vinod Koul wrote:
> On Mon, Nov 02, 2015 at 12:07:27PM +0000, Mark Brown wrote:
> > On Mon, Nov 02, 2015 at 03:41:08PM +0530, Vinod Koul wrote:

> > > It is similar but sst_machines structure is entirely different. In Skylake
> > > case we only need machine entry name and rest of the information is coming

> > So what exactly is the "machine entry name" supposed to be here and why
> > don't we use any board specific information?

> For this case the machine entry name is "skl_alc286s_i2s", which is the RT
> ALC 286 codec combination on Skylake.
> The SKL machine driver needs this as platform device and we create it here

> > +static struct sst_machines sst_skl_devdata[] = {
> > +	{ "INT343A", "skl_alc286s_i2s" },
> > +};
> 
> This says for SKL, with codec ID "INT343A" create "skl_alc286s_i2s" machine
> platform device
> For different codec combination we can use codec ACPI names to match

I'm having a hard time seeing the difference between this and what's
going on in sst-acpi.  They seem to be doing the same thing in slightly
different ways, they both match tables of CODEC IDs to machine driver
names with the distinction being that this doesn't provide a firmware
filename whereas sst-acpi does but the mechanics of mapping a CODEC to a
machine driver seem otherwise the same.

> > > I am not sure I follow the comment on user, the SKL driver here is user in
> > > this

> > There are no machines defined for this.

> sound/soc/intel/boards/skl_rt286.c

Ugh, the machines table is *really* buried at the bottom of the file
here :(

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

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

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Keyon]

On 2015年11月03日 19:24, Mark Brown wrote:
> On Mon, Nov 02, 2015 at 09:02:29PM +0530, Vinod Koul wrote:
>> On Mon, Nov 02, 2015 at 12:07:27PM +0000, Mark Brown wrote:
>>> On Mon, Nov 02, 2015 at 03:41:08PM +0530, Vinod Koul wrote:
>
>>>> It is similar but sst_machines structure is entirely different. In Skylake
>>>> case we only need machine entry name and rest of the information is coming
>
>>> So what exactly is the "machine entry name" supposed to be here and why
>>> don't we use any board specific information?
>
>> For this case the machine entry name is "skl_alc286s_i2s", which is the RT
>> ALC 286 codec combination on Skylake.
>> The SKL machine driver needs this as platform device and we create it here
>
>>> +static struct sst_machines sst_skl_devdata[] = {
>>> +	{ "INT343A", "skl_alc286s_i2s" },
>>> +};
>>
>> This says for SKL, with codec ID "INT343A" create "skl_alc286s_i2s" machine
>> platform device
>> For different codec combination we can use codec ACPI names to match
>
> I'm having a hard time seeing the difference between this and what's
> going on in sst-acpi.  They seem to be doing the same thing in slightly
> different ways, they both match tables of CODEC IDs to machine driver
> names with the distinction being that this doesn't provide a firmware
> filename whereas sst-acpi does but the mechanics of mapping a CODEC to a
> machine driver seem otherwise the same.

agree that it may be better if we can reuse(or merge with) sst driver 
(intel/common/sst-acpi.c) for SKL.

thanks,
~Keyon

>
>>>> I am not sure I follow the comment on user, the SKL driver here is user in
>>>> this
>
>>> There are no machines defined for this.
>
>> sound/soc/intel/boards/skl_rt286.c
>
> Ugh, the machines table is *really* buried at the bottom of the file
> here :(
>
>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Vinod Koul]

On Tue, Nov 03, 2015 at 08:29:02PM +0800, Keyon wrote:
> >I'm having a hard time seeing the difference between this and what's
> >going on in sst-acpi.  They seem to be doing the same thing in slightly
> >different ways, they both match tables of CODEC IDs to machine driver
> >names with the distinction being that this doesn't provide a firmware
> >filename whereas sst-acpi does but the mechanics of mapping a CODEC to a
> >machine driver seem otherwise the same.
> 
> agree that it may be better if we can reuse(or merge with) sst
> driver (intel/common/sst-acpi.c) for SKL.

all drivers use different sst_machine structure... and data required is not
same

-- 
~Vinod

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Vinod Koul]

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

On Tue, Nov 03, 2015 at 11:24:58AM +0000, Mark Brown wrote:
> > For this case the machine entry name is "skl_alc286s_i2s", which is the RT
> > ALC 286 codec combination on Skylake.
> > The SKL machine driver needs this as platform device and we create it here
> 
> > > +static struct sst_machines sst_skl_devdata[] = {
> > > +	{ "INT343A", "skl_alc286s_i2s" },
> > > +};
> > 
> > This says for SKL, with codec ID "INT343A" create "skl_alc286s_i2s" machine
> > platform device
> > For different codec combination we can use codec ACPI names to match
> 
> I'm having a hard time seeing the difference between this and what's
> going on in sst-acpi.  They seem to be doing the same thing in slightly
> different ways, they both match tables of CODEC IDs to machine driver
> names with the distinction being that this doesn't provide a firmware
> filename whereas sst-acpi does but the mechanics of mapping a CODEC to a
> machine driver seem otherwise the same.

I don't disagree with your observation, the code does same stuff and only
difference is the data we require here, so commonizing becomes tricky

> > > > I am not sure I follow the comment on user, the SKL driver here is user in
> > > > this
> 
> > > There are no machines defined for this.
> 
> > sound/soc/intel/boards/skl_rt286.c
> 
> Ugh, the machines table is *really* buried at the bottom of the file
> here :(

Ah sorry for that, I can add a comment here and send an update

-- 
~Vinod

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Mark Brown]

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

On Tue, Nov 03, 2015 at 11:27:08PM +0530, Vinod Koul wrote:
> On Tue, Nov 03, 2015 at 11:24:58AM +0000, Mark Brown wrote:

> > I'm having a hard time seeing the difference between this and what's
> > going on in sst-acpi.  They seem to be doing the same thing in slightly
> > different ways, they both match tables of CODEC IDs to machine driver
> > names with the distinction being that this doesn't provide a firmware
> > filename whereas sst-acpi does but the mechanics of mapping a CODEC to a
> > machine driver seem otherwise the same.

> I don't disagree with your observation, the code does same stuff and only
> difference is the data we require here, so commonizing becomes tricky

The only difference in data appears to be a firmware file name?

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

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

Re: [PATCH 1/3] ASoC: Intel: Skylake: fix missing machine device creation

[Vinod Koul]

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

On Wed, Nov 04, 2015 at 02:29:58PM +0000, Mark Brown wrote:
> On Tue, Nov 03, 2015 at 11:27:08PM +0530, Vinod Koul wrote:
> > On Tue, Nov 03, 2015 at 11:24:58AM +0000, Mark Brown wrote:
> 
> > > I'm having a hard time seeing the difference between this and what's
> > > going on in sst-acpi.  They seem to be doing the same thing in slightly
> > > different ways, they both match tables of CODEC IDs to machine driver
> > > names with the distinction being that this doesn't provide a firmware
> > > filename whereas sst-acpi does but the mechanics of mapping a CODEC to a
> > > machine driver seem otherwise the same.
> 
> > I don't disagree with your observation, the code does same stuff and only
> > difference is the data we require here, so commonizing becomes tricky
> 
> The only difference in data appears to be a firmware file name?
The one atom code uses is quite different, but i did manage to create a
global one and ignore those in other places

I will send updated patches moving all to common apci match code and then
add SKL to common code

Thanks
-- 
~Vinod

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[PATCH 2/3] ASoC: Intel: Skylake: Fix to cleanup if skl_sst_dsp_init fails

[Vinod Koul]

From: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>

This patch fixes the below warning reported by Dan by invoking
skl_sst_dsp_cleanup() in cleanup path on error and not bailing out

	sound/soc/intel/skylake/skl-sst.c:270 skl_sst_dsp_init()
	info: ignoring unreachable code.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl-sst.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c
index 3b83dc99f1d4..5c5a244942ef 100644
--- a/sound/soc/intel/skylake/skl-sst.c
+++ b/sound/soc/intel/skylake/skl-sst.c
@@ -259,15 +259,16 @@ int skl_sst_dsp_init(struct device *dev, void __iomem *mmio_base, int irq,
 	ret = sst->fw_ops.load_fw(sst);
 	if (ret < 0) {
 		dev_err(dev, "Load base fw failed : %d", ret);
-		return ret;
+		goto cleanup;
 	}
 
 	if (dsp)
 		*dsp = skl;
 
-	return 0;
+	return ret;
 
-	skl_ipc_free(&skl->ipc);
+cleanup:
+	skl_sst_dsp_cleanup(dev, skl);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(skl_sst_dsp_init);
-- 
1.9.1

Applied "ASoC: Intel: Skylake: Fix to cleanup if skl_sst_dsp_init fails" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: Skylake: Fix to cleanup if skl_sst_dsp_init fails

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 44b69590a64f1f2694fd0dd38cd3efbe751f9a8a Mon Sep 17 00:00:00 2001
From: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
Date: Fri, 30 Oct 2015 20:34:19 +0530
Subject: [PATCH] ASoC: Intel: Skylake: Fix to cleanup if skl_sst_dsp_init
 fails

This patch fixes the below warning reported by Dan by invoking
skl_sst_dsp_cleanup() in cleanup path on error and not bailing out

	sound/soc/intel/skylake/skl-sst.c:270 skl_sst_dsp_init()
	info: ignoring unreachable code.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/skylake/skl-sst.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-sst.c b/sound/soc/intel/skylake/skl-sst.c
index 3b83dc9..5c5a244 100644
--- a/sound/soc/intel/skylake/skl-sst.c
+++ b/sound/soc/intel/skylake/skl-sst.c
@@ -259,15 +259,16 @@ int skl_sst_dsp_init(struct device *dev, void __iomem *mmio_base, int irq,
 	ret = sst->fw_ops.load_fw(sst);
 	if (ret < 0) {
 		dev_err(dev, "Load base fw failed : %d", ret);
-		return ret;
+		goto cleanup;
 	}
 
 	if (dsp)
 		*dsp = skl;
 
-	return 0;
+	return ret;
 
-	skl_ipc_free(&skl->ipc);
+cleanup:
+	skl_sst_dsp_cleanup(dev, skl);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(skl_sst_dsp_init);
-- 
2.6.1

[PATCH 3/3] ASoC: Intel: Skylake: Fix substream dereference before check

[Vinod Koul]

Smatch warns that we dereferenced substream before check, so fix
this by initializing ebus after the check

sound/soc/intel/skylake/skl-pcm.c:802 skl_get_position()
        warn: variable dereferenced before check 'substream->runtime'

Reported by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 2517ec576ffc..e652d58bd9a9 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -807,7 +807,7 @@ static unsigned int skl_get_position(struct hdac_ext_stream *hstream,
 {
 	struct hdac_stream *hstr = hdac_stream(hstream);
 	struct snd_pcm_substream *substream = hstr->substream;
-	struct hdac_ext_bus *ebus = get_bus_ctx(substream);
+	struct hdac_ext_bus *ebus;
 	unsigned int pos;
 	int delay;
 
@@ -818,6 +818,7 @@ static unsigned int skl_get_position(struct hdac_ext_stream *hstream,
 		pos = 0;
 
 	if (substream->runtime) {
+		ebus = get_bus_ctx(substream);
 		delay = skl_get_delay_from_lpib(ebus, hstream, pos)
 						 + codec_delay;
 		substream->runtime->delay += delay;
-- 
1.9.1

Applied "ASoC: Intel: Skylake: Fix substream dereference before check" to the asoc tree

[Mark Brown]

The patch

   ASoC: Intel: Skylake: Fix substream dereference before check

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 5802b6ab6cb499739beb9de8643d2b7b2c2ae00a Mon Sep 17 00:00:00 2001
From: Vinod Koul <vinod.koul@intel.com>
Date: Fri, 30 Oct 2015 20:34:20 +0530
Subject: [PATCH] ASoC: Intel: Skylake: Fix substream dereference before check

Smatch warns that we dereferenced substream before check, so fix
this by initializing ebus after the check

sound/soc/intel/skylake/skl-pcm.c:802 skl_get_position()
        warn: variable dereferenced before check 'substream->runtime'

Reported by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/intel/skylake/skl-pcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 2517ec5..e652d58 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -807,7 +807,7 @@ static unsigned int skl_get_position(struct hdac_ext_stream *hstream,
 {
 	struct hdac_stream *hstr = hdac_stream(hstream);
 	struct snd_pcm_substream *substream = hstr->substream;
-	struct hdac_ext_bus *ebus = get_bus_ctx(substream);
+	struct hdac_ext_bus *ebus;
 	unsigned int pos;
 	int delay;
 
@@ -818,6 +818,7 @@ static unsigned int skl_get_position(struct hdac_ext_stream *hstream,
 		pos = 0;
 
 	if (substream->runtime) {
+		ebus = get_bus_ctx(substream);
 		delay = skl_get_delay_from_lpib(ebus, hstream, pos)
 						 + codec_delay;
 		substream->runtime->delay += delay;
-- 
2.6.1

[PATCH 0/3] ASoC: Intel: Skylake: Some more fixes

[Vinod Koul]

This would hopefully be last round of fixes required to have SKL completely
functional on upstream.

First patch is actually only one required for that which adds misisng
mahcine entry for SKL boards with RT286.
Rest of the two are fixes for static checker warnings reported by folks.

Jeeja KP (1):
  ASoC: Intel: Skylake: fix missing machine device creation

Subhransu S. Prusty (1):
  ASoC: Intel: Skylake: Fix to cleanup if skl_sst_dsp_init fails

Vinod Koul (1):
  ASoC: Intel: Skylake: Fix substream dereference before check

 sound/soc/intel/skylake/skl-pcm.c |  3 +-
 sound/soc/intel/skylake/skl-sst.c |  7 ++--
 sound/soc/intel/skylake/skl.c     | 79 ++++++++++++++++++++++++++++++++++++++-
 sound/soc/intel/skylake/skl.h     |  1 +
 4 files changed, 84 insertions(+), 6 deletions(-)

-- 
1.9.1