All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] soundwire: intel: transition to 3 steps initialization
@ 2020-05-31 18:20 ` Bard Liao
  0 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:20 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

This series is to split the original "soundwire: intel: transition to 3
steps initialization" patch into different patches for better review.
It also address comments from Vinod.

Pierre-Louis Bossart (6):
  soundwire: intel: cleanups for indirections/logs
  soundwire: intel: clarify drvdata and remove more indirections
  soundwire: intel_init: remove useless test
  soundwire: intel_init: use devm_ allocation
  soundwire: intel_init: pass link information as platform data
  soundwire: intel: transition to 3 steps initialization

 drivers/soundwire/intel.c      | 138 +++++++++--------
 drivers/soundwire/intel.h      |  13 ++
 drivers/soundwire/intel_init.c | 261 ++++++++++++++++++++++++++-------
 3 files changed, 295 insertions(+), 117 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 0/6] soundwire: intel: transition to 3 steps initialization
@ 2020-05-31 18:20 ` Bard Liao
  0 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:20 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: pierre-louis.bossart, vinod.koul, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

This series is to split the original "soundwire: intel: transition to 3
steps initialization" patch into different patches for better review.
It also address comments from Vinod.

Pierre-Louis Bossart (6):
  soundwire: intel: cleanups for indirections/logs
  soundwire: intel: clarify drvdata and remove more indirections
  soundwire: intel_init: remove useless test
  soundwire: intel_init: use devm_ allocation
  soundwire: intel_init: pass link information as platform data
  soundwire: intel: transition to 3 steps initialization

 drivers/soundwire/intel.c      | 138 +++++++++--------
 drivers/soundwire/intel.h      |  13 ++
 drivers/soundwire/intel_init.c | 261 ++++++++++++++++++++++++++-------
 3 files changed, 295 insertions(+), 117 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/6] soundwire: intel: cleanups for indirections/logs
  2020-05-31 18:20 ` Bard Liao
@ 2020-05-31 18:20   ` Bard Liao
  -1 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:20 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The code can be simplified a bit to have a more consistent use of
'dev' and 'bus', as well as move definitions around. This will help
make the major changes in follow-up patches easier to review.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c      | 68 +++++++++++++++++-----------------
 drivers/soundwire/intel.h      | 11 ++++++
 drivers/soundwire/intel_init.c |  1 +
 3 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 1eda63f488f5..10e55c708694 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -92,23 +92,12 @@
 #define SDW_ALH_STRMZCFG_DMAT		GENMASK(7, 0)
 #define SDW_ALH_STRMZCFG_CHN		GENMASK(19, 16)
 
-#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE	BIT(1)
-
 enum intel_pdi_type {
 	INTEL_PDI_IN = 0,
 	INTEL_PDI_OUT = 1,
 	INTEL_PDI_BD = 2,
 };
 
-struct sdw_intel {
-	struct sdw_cdns cdns;
-	int instance;
-	struct sdw_intel_link_res *link_res;
-#ifdef CONFIG_DEBUG_FS
-	struct dentry *debugfs;
-#endif
-};
-
 #define cdns_to_intel(_cdns) container_of(_cdns, struct sdw_intel, cdns)
 
 /*
@@ -1083,41 +1072,47 @@ static int intel_init(struct sdw_intel *sdw)
 /*
  * probe and init
  */
-static int intel_probe(struct platform_device *pdev)
+static int intel_master_probe(struct platform_device *pdev)
 {
 	struct sdw_cdns_stream_config config;
+	struct device *dev = &pdev->dev;
 	struct sdw_intel *sdw;
+	struct sdw_bus *bus;
 	int ret;
 
-	sdw = devm_kzalloc(&pdev->dev, sizeof(*sdw), GFP_KERNEL);
+	sdw = devm_kzalloc(dev, sizeof(*sdw), GFP_KERNEL);
 	if (!sdw)
 		return -ENOMEM;
 
+	bus = &sdw->cdns.bus;
+
 	sdw->instance = pdev->id;
-	sdw->link_res = dev_get_platdata(&pdev->dev);
-	sdw->cdns.dev = &pdev->dev;
+	sdw->link_res = dev_get_platdata(dev);
+	sdw->cdns.dev = dev;
 	sdw->cdns.registers = sdw->link_res->registers;
 	sdw->cdns.instance = sdw->instance;
 	sdw->cdns.msg_count = 0;
-	sdw->cdns.bus.link_id = pdev->id;
+	bus->link_id = pdev->id;
 
 	sdw_cdns_probe(&sdw->cdns);
 
 	/* Set property read ops */
 	sdw_intel_ops.read_prop = intel_prop_read;
-	sdw->cdns.bus.ops = &sdw_intel_ops;
+	bus->ops = &sdw_intel_ops;
 
+	/* set driver data, accessed by snd_soc_dai_get_drvdata() */
 	platform_set_drvdata(pdev, sdw);
 
-	ret = sdw_bus_master_add(&sdw->cdns.bus, &pdev->dev, pdev->dev.fwnode);
+	ret = sdw_bus_master_add(bus, dev, dev->fwnode);
 	if (ret) {
-		dev_err(&pdev->dev, "sdw_bus_master_add fail: %d\n", ret);
+		dev_err(dev, "sdw_bus_master_add fail: %d\n", ret);
 		return ret;
 	}
 
-	if (sdw->cdns.bus.prop.hw_disabled) {
-		dev_info(&pdev->dev, "SoundWire master %d is disabled, ignoring\n",
-			 sdw->cdns.bus.link_id);
+	if (bus->prop.hw_disabled) {
+		dev_info(dev,
+			 "SoundWire master %d is disabled, will be ignored\n",
+			 bus->link_id);
 		return 0;
 	}
 
@@ -1139,28 +1134,28 @@ static int intel_probe(struct platform_device *pdev)
 				   sdw_cdns_irq, sdw_cdns_thread,
 				   IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
 	if (ret < 0) {
-		dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n",
+		dev_err(dev, "unable to grab IRQ %d, disabling device\n",
 			sdw->link_res->irq);
 		goto err_init;
 	}
 
 	ret = sdw_cdns_enable_interrupt(&sdw->cdns, true);
 	if (ret < 0) {
-		dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
+		dev_err(dev, "cannot enable interrupts\n");
 		goto err_init;
 	}
 
 	ret = sdw_cdns_exit_reset(&sdw->cdns);
 	if (ret < 0) {
-		dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n");
+		dev_err(dev, "unable to exit bus reset sequence\n");
 		goto err_interrupt;
 	}
 
 	/* Register DAIs */
 	ret = intel_register_dai(sdw);
 	if (ret) {
-		dev_err(sdw->cdns.dev, "DAI registration failed: %d\n", ret);
-		snd_soc_unregister_component(sdw->cdns.dev);
+		dev_err(dev, "DAI registration failed: %d\n", ret);
+		snd_soc_unregister_component(dev);
 		goto err_interrupt;
 	}
 
@@ -1172,33 +1167,36 @@ static int intel_probe(struct platform_device *pdev)
 	sdw_cdns_enable_interrupt(&sdw->cdns, false);
 	free_irq(sdw->link_res->irq, sdw);
 err_init:
-	sdw_bus_master_delete(&sdw->cdns.bus);
+	sdw_bus_master_delete(bus);
 	return ret;
 }
 
-static int intel_remove(struct platform_device *pdev)
+static int intel_master_remove(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct sdw_intel *sdw;
+	struct sdw_bus *bus;
 
 	sdw = platform_get_drvdata(pdev);
 
-	if (!sdw->cdns.bus.prop.hw_disabled) {
+	bus = &sdw->cdns.bus;
+
+	if (!bus->prop.hw_disabled) {
 		intel_debugfs_exit(sdw);
 		sdw_cdns_enable_interrupt(&sdw->cdns, false);
 		free_irq(sdw->link_res->irq, sdw);
-		snd_soc_unregister_component(sdw->cdns.dev);
+		snd_soc_unregister_component(dev);
 	}
-	sdw_bus_master_delete(&sdw->cdns.bus);
+	sdw_bus_master_delete(bus);
 
 	return 0;
 }
 
 static struct platform_driver sdw_intel_drv = {
-	.probe = intel_probe,
-	.remove = intel_remove,
+	.probe = intel_master_probe,
+	.remove = intel_master_remove,
 	.driver = {
 		.name = "int-sdw",
-
 	},
 };
 
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 7b4af8018e1a..001968c362cd 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -27,4 +27,15 @@ struct sdw_intel_link_res {
 	struct device *dev;
 };
 
+struct sdw_intel {
+	struct sdw_cdns cdns;
+	int instance;
+	struct sdw_intel_link_res *link_res;
+#ifdef CONFIG_DEBUG_FS
+	struct dentry *debugfs;
+#endif
+};
+
+#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE      BIT(1)
+
 #endif /* __SDW_INTEL_LOCAL_H */
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 947345d5c960..de0405359db5 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/soundwire/sdw_intel.h>
+#include "cadence_master.h"
 #include "intel.h"
 
 #define SDW_LINK_TYPE		4 /* from Intel ACPI documentation */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 1/6] soundwire: intel: cleanups for indirections/logs
@ 2020-05-31 18:20   ` Bard Liao
  0 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:20 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: pierre-louis.bossart, vinod.koul, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The code can be simplified a bit to have a more consistent use of
'dev' and 'bus', as well as move definitions around. This will help
make the major changes in follow-up patches easier to review.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c      | 68 +++++++++++++++++-----------------
 drivers/soundwire/intel.h      | 11 ++++++
 drivers/soundwire/intel_init.c |  1 +
 3 files changed, 45 insertions(+), 35 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 1eda63f488f5..10e55c708694 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -92,23 +92,12 @@
 #define SDW_ALH_STRMZCFG_DMAT		GENMASK(7, 0)
 #define SDW_ALH_STRMZCFG_CHN		GENMASK(19, 16)
 
-#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE	BIT(1)
-
 enum intel_pdi_type {
 	INTEL_PDI_IN = 0,
 	INTEL_PDI_OUT = 1,
 	INTEL_PDI_BD = 2,
 };
 
-struct sdw_intel {
-	struct sdw_cdns cdns;
-	int instance;
-	struct sdw_intel_link_res *link_res;
-#ifdef CONFIG_DEBUG_FS
-	struct dentry *debugfs;
-#endif
-};
-
 #define cdns_to_intel(_cdns) container_of(_cdns, struct sdw_intel, cdns)
 
 /*
@@ -1083,41 +1072,47 @@ static int intel_init(struct sdw_intel *sdw)
 /*
  * probe and init
  */
-static int intel_probe(struct platform_device *pdev)
+static int intel_master_probe(struct platform_device *pdev)
 {
 	struct sdw_cdns_stream_config config;
+	struct device *dev = &pdev->dev;
 	struct sdw_intel *sdw;
+	struct sdw_bus *bus;
 	int ret;
 
-	sdw = devm_kzalloc(&pdev->dev, sizeof(*sdw), GFP_KERNEL);
+	sdw = devm_kzalloc(dev, sizeof(*sdw), GFP_KERNEL);
 	if (!sdw)
 		return -ENOMEM;
 
+	bus = &sdw->cdns.bus;
+
 	sdw->instance = pdev->id;
-	sdw->link_res = dev_get_platdata(&pdev->dev);
-	sdw->cdns.dev = &pdev->dev;
+	sdw->link_res = dev_get_platdata(dev);
+	sdw->cdns.dev = dev;
 	sdw->cdns.registers = sdw->link_res->registers;
 	sdw->cdns.instance = sdw->instance;
 	sdw->cdns.msg_count = 0;
-	sdw->cdns.bus.link_id = pdev->id;
+	bus->link_id = pdev->id;
 
 	sdw_cdns_probe(&sdw->cdns);
 
 	/* Set property read ops */
 	sdw_intel_ops.read_prop = intel_prop_read;
-	sdw->cdns.bus.ops = &sdw_intel_ops;
+	bus->ops = &sdw_intel_ops;
 
+	/* set driver data, accessed by snd_soc_dai_get_drvdata() */
 	platform_set_drvdata(pdev, sdw);
 
-	ret = sdw_bus_master_add(&sdw->cdns.bus, &pdev->dev, pdev->dev.fwnode);
+	ret = sdw_bus_master_add(bus, dev, dev->fwnode);
 	if (ret) {
-		dev_err(&pdev->dev, "sdw_bus_master_add fail: %d\n", ret);
+		dev_err(dev, "sdw_bus_master_add fail: %d\n", ret);
 		return ret;
 	}
 
-	if (sdw->cdns.bus.prop.hw_disabled) {
-		dev_info(&pdev->dev, "SoundWire master %d is disabled, ignoring\n",
-			 sdw->cdns.bus.link_id);
+	if (bus->prop.hw_disabled) {
+		dev_info(dev,
+			 "SoundWire master %d is disabled, will be ignored\n",
+			 bus->link_id);
 		return 0;
 	}
 
@@ -1139,28 +1134,28 @@ static int intel_probe(struct platform_device *pdev)
 				   sdw_cdns_irq, sdw_cdns_thread,
 				   IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
 	if (ret < 0) {
-		dev_err(sdw->cdns.dev, "unable to grab IRQ %d, disabling device\n",
+		dev_err(dev, "unable to grab IRQ %d, disabling device\n",
 			sdw->link_res->irq);
 		goto err_init;
 	}
 
 	ret = sdw_cdns_enable_interrupt(&sdw->cdns, true);
 	if (ret < 0) {
-		dev_err(sdw->cdns.dev, "cannot enable interrupts\n");
+		dev_err(dev, "cannot enable interrupts\n");
 		goto err_init;
 	}
 
 	ret = sdw_cdns_exit_reset(&sdw->cdns);
 	if (ret < 0) {
-		dev_err(sdw->cdns.dev, "unable to exit bus reset sequence\n");
+		dev_err(dev, "unable to exit bus reset sequence\n");
 		goto err_interrupt;
 	}
 
 	/* Register DAIs */
 	ret = intel_register_dai(sdw);
 	if (ret) {
-		dev_err(sdw->cdns.dev, "DAI registration failed: %d\n", ret);
-		snd_soc_unregister_component(sdw->cdns.dev);
+		dev_err(dev, "DAI registration failed: %d\n", ret);
+		snd_soc_unregister_component(dev);
 		goto err_interrupt;
 	}
 
@@ -1172,33 +1167,36 @@ static int intel_probe(struct platform_device *pdev)
 	sdw_cdns_enable_interrupt(&sdw->cdns, false);
 	free_irq(sdw->link_res->irq, sdw);
 err_init:
-	sdw_bus_master_delete(&sdw->cdns.bus);
+	sdw_bus_master_delete(bus);
 	return ret;
 }
 
-static int intel_remove(struct platform_device *pdev)
+static int intel_master_remove(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct sdw_intel *sdw;
+	struct sdw_bus *bus;
 
 	sdw = platform_get_drvdata(pdev);
 
-	if (!sdw->cdns.bus.prop.hw_disabled) {
+	bus = &sdw->cdns.bus;
+
+	if (!bus->prop.hw_disabled) {
 		intel_debugfs_exit(sdw);
 		sdw_cdns_enable_interrupt(&sdw->cdns, false);
 		free_irq(sdw->link_res->irq, sdw);
-		snd_soc_unregister_component(sdw->cdns.dev);
+		snd_soc_unregister_component(dev);
 	}
-	sdw_bus_master_delete(&sdw->cdns.bus);
+	sdw_bus_master_delete(bus);
 
 	return 0;
 }
 
 static struct platform_driver sdw_intel_drv = {
-	.probe = intel_probe,
-	.remove = intel_remove,
+	.probe = intel_master_probe,
+	.remove = intel_master_remove,
 	.driver = {
 		.name = "int-sdw",
-
 	},
 };
 
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 7b4af8018e1a..001968c362cd 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -27,4 +27,15 @@ struct sdw_intel_link_res {
 	struct device *dev;
 };
 
+struct sdw_intel {
+	struct sdw_cdns cdns;
+	int instance;
+	struct sdw_intel_link_res *link_res;
+#ifdef CONFIG_DEBUG_FS
+	struct dentry *debugfs;
+#endif
+};
+
+#define SDW_INTEL_QUIRK_MASK_BUS_DISABLE      BIT(1)
+
 #endif /* __SDW_INTEL_LOCAL_H */
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index 947345d5c960..de0405359db5 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/soundwire/sdw_intel.h>
+#include "cadence_master.h"
 #include "intel.h"
 
 #define SDW_LINK_TYPE		4 /* from Intel ACPI documentation */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/6] soundwire: intel: clarify drvdata and remove more indirections
  2020-05-31 18:20 ` Bard Liao
@ 2020-05-31 18:20   ` Bard Liao
  -1 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:20 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The use of drvdata mixes two structures. There was no harm the first
structure is embedded as the first element of the second, but that's
not good. Make sure all drvdata is based on the 'sdw_cdns' structure.

While we are at it, remove indirections for 'dev' and 'cdns' to make
the code more readable.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 10e55c708694..ef1196ea93a4 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1077,6 +1077,7 @@ static int intel_master_probe(struct platform_device *pdev)
 	struct sdw_cdns_stream_config config;
 	struct device *dev = &pdev->dev;
 	struct sdw_intel *sdw;
+	struct sdw_cdns *cdns;
 	struct sdw_bus *bus;
 	int ret;
 
@@ -1084,24 +1085,26 @@ static int intel_master_probe(struct platform_device *pdev)
 	if (!sdw)
 		return -ENOMEM;
 
-	bus = &sdw->cdns.bus;
+	cdns = &sdw->cdns;
+	bus = &cdns->bus;
 
 	sdw->instance = pdev->id;
 	sdw->link_res = dev_get_platdata(dev);
-	sdw->cdns.dev = dev;
-	sdw->cdns.registers = sdw->link_res->registers;
-	sdw->cdns.instance = sdw->instance;
-	sdw->cdns.msg_count = 0;
+	cdns->dev = dev;
+	cdns->registers = sdw->link_res->registers;
+	cdns->instance = sdw->instance;
+	cdns->msg_count = 0;
+
 	bus->link_id = pdev->id;
 
-	sdw_cdns_probe(&sdw->cdns);
+	sdw_cdns_probe(cdns);
 
 	/* Set property read ops */
 	sdw_intel_ops.read_prop = intel_prop_read;
 	bus->ops = &sdw_intel_ops;
 
 	/* set driver data, accessed by snd_soc_dai_get_drvdata() */
-	platform_set_drvdata(pdev, sdw);
+	dev_set_drvdata(dev, cdns);
 
 	ret = sdw_bus_master_add(bus, dev, dev->fwnode);
 	if (ret) {
@@ -1123,7 +1126,7 @@ static int intel_master_probe(struct platform_device *pdev)
 
 	/* Read the PDI config and initialize cadence PDI */
 	intel_pdi_init(sdw, &config);
-	ret = sdw_cdns_pdi_init(&sdw->cdns, config);
+	ret = sdw_cdns_pdi_init(cdns, config);
 	if (ret)
 		goto err_init;
 
@@ -1132,20 +1135,20 @@ static int intel_master_probe(struct platform_device *pdev)
 	/* Acquire IRQ */
 	ret = request_threaded_irq(sdw->link_res->irq,
 				   sdw_cdns_irq, sdw_cdns_thread,
-				   IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
+				   IRQF_SHARED, KBUILD_MODNAME, cdns);
 	if (ret < 0) {
 		dev_err(dev, "unable to grab IRQ %d, disabling device\n",
 			sdw->link_res->irq);
 		goto err_init;
 	}
 
-	ret = sdw_cdns_enable_interrupt(&sdw->cdns, true);
+	ret = sdw_cdns_enable_interrupt(cdns, true);
 	if (ret < 0) {
 		dev_err(dev, "cannot enable interrupts\n");
 		goto err_init;
 	}
 
-	ret = sdw_cdns_exit_reset(&sdw->cdns);
+	ret = sdw_cdns_exit_reset(cdns);
 	if (ret < 0) {
 		dev_err(dev, "unable to exit bus reset sequence\n");
 		goto err_interrupt;
@@ -1164,7 +1167,7 @@ static int intel_master_probe(struct platform_device *pdev)
 	return 0;
 
 err_interrupt:
-	sdw_cdns_enable_interrupt(&sdw->cdns, false);
+	sdw_cdns_enable_interrupt(cdns, false);
 	free_irq(sdw->link_res->irq, sdw);
 err_init:
 	sdw_bus_master_delete(bus);
@@ -1174,16 +1177,13 @@ static int intel_master_probe(struct platform_device *pdev)
 static int intel_master_remove(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct sdw_intel *sdw;
-	struct sdw_bus *bus;
-
-	sdw = platform_get_drvdata(pdev);
-
-	bus = &sdw->cdns.bus;
+	struct sdw_cdns *cdns = dev_get_drvdata(dev);
+	struct sdw_intel *sdw = cdns_to_intel(cdns);
+	struct sdw_bus *bus = &cdns->bus;
 
 	if (!bus->prop.hw_disabled) {
 		intel_debugfs_exit(sdw);
-		sdw_cdns_enable_interrupt(&sdw->cdns, false);
+		sdw_cdns_enable_interrupt(cdns, false);
 		free_irq(sdw->link_res->irq, sdw);
 		snd_soc_unregister_component(dev);
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/6] soundwire: intel: clarify drvdata and remove more indirections
@ 2020-05-31 18:20   ` Bard Liao
  0 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:20 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: pierre-louis.bossart, vinod.koul, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The use of drvdata mixes two structures. There was no harm the first
structure is embedded as the first element of the second, but that's
not good. Make sure all drvdata is based on the 'sdw_cdns' structure.

While we are at it, remove indirections for 'dev' and 'cdns' to make
the code more readable.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index 10e55c708694..ef1196ea93a4 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1077,6 +1077,7 @@ static int intel_master_probe(struct platform_device *pdev)
 	struct sdw_cdns_stream_config config;
 	struct device *dev = &pdev->dev;
 	struct sdw_intel *sdw;
+	struct sdw_cdns *cdns;
 	struct sdw_bus *bus;
 	int ret;
 
@@ -1084,24 +1085,26 @@ static int intel_master_probe(struct platform_device *pdev)
 	if (!sdw)
 		return -ENOMEM;
 
-	bus = &sdw->cdns.bus;
+	cdns = &sdw->cdns;
+	bus = &cdns->bus;
 
 	sdw->instance = pdev->id;
 	sdw->link_res = dev_get_platdata(dev);
-	sdw->cdns.dev = dev;
-	sdw->cdns.registers = sdw->link_res->registers;
-	sdw->cdns.instance = sdw->instance;
-	sdw->cdns.msg_count = 0;
+	cdns->dev = dev;
+	cdns->registers = sdw->link_res->registers;
+	cdns->instance = sdw->instance;
+	cdns->msg_count = 0;
+
 	bus->link_id = pdev->id;
 
-	sdw_cdns_probe(&sdw->cdns);
+	sdw_cdns_probe(cdns);
 
 	/* Set property read ops */
 	sdw_intel_ops.read_prop = intel_prop_read;
 	bus->ops = &sdw_intel_ops;
 
 	/* set driver data, accessed by snd_soc_dai_get_drvdata() */
-	platform_set_drvdata(pdev, sdw);
+	dev_set_drvdata(dev, cdns);
 
 	ret = sdw_bus_master_add(bus, dev, dev->fwnode);
 	if (ret) {
@@ -1123,7 +1126,7 @@ static int intel_master_probe(struct platform_device *pdev)
 
 	/* Read the PDI config and initialize cadence PDI */
 	intel_pdi_init(sdw, &config);
-	ret = sdw_cdns_pdi_init(&sdw->cdns, config);
+	ret = sdw_cdns_pdi_init(cdns, config);
 	if (ret)
 		goto err_init;
 
@@ -1132,20 +1135,20 @@ static int intel_master_probe(struct platform_device *pdev)
 	/* Acquire IRQ */
 	ret = request_threaded_irq(sdw->link_res->irq,
 				   sdw_cdns_irq, sdw_cdns_thread,
-				   IRQF_SHARED, KBUILD_MODNAME, &sdw->cdns);
+				   IRQF_SHARED, KBUILD_MODNAME, cdns);
 	if (ret < 0) {
 		dev_err(dev, "unable to grab IRQ %d, disabling device\n",
 			sdw->link_res->irq);
 		goto err_init;
 	}
 
-	ret = sdw_cdns_enable_interrupt(&sdw->cdns, true);
+	ret = sdw_cdns_enable_interrupt(cdns, true);
 	if (ret < 0) {
 		dev_err(dev, "cannot enable interrupts\n");
 		goto err_init;
 	}
 
-	ret = sdw_cdns_exit_reset(&sdw->cdns);
+	ret = sdw_cdns_exit_reset(cdns);
 	if (ret < 0) {
 		dev_err(dev, "unable to exit bus reset sequence\n");
 		goto err_interrupt;
@@ -1164,7 +1167,7 @@ static int intel_master_probe(struct platform_device *pdev)
 	return 0;
 
 err_interrupt:
-	sdw_cdns_enable_interrupt(&sdw->cdns, false);
+	sdw_cdns_enable_interrupt(cdns, false);
 	free_irq(sdw->link_res->irq, sdw);
 err_init:
 	sdw_bus_master_delete(bus);
@@ -1174,16 +1177,13 @@ static int intel_master_probe(struct platform_device *pdev)
 static int intel_master_remove(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct sdw_intel *sdw;
-	struct sdw_bus *bus;
-
-	sdw = platform_get_drvdata(pdev);
-
-	bus = &sdw->cdns.bus;
+	struct sdw_cdns *cdns = dev_get_drvdata(dev);
+	struct sdw_intel *sdw = cdns_to_intel(cdns);
+	struct sdw_bus *bus = &cdns->bus;
 
 	if (!bus->prop.hw_disabled) {
 		intel_debugfs_exit(sdw);
-		sdw_cdns_enable_interrupt(&sdw->cdns, false);
+		sdw_cdns_enable_interrupt(cdns, false);
 		free_irq(sdw->link_res->irq, sdw);
 		snd_soc_unregister_component(dev);
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/6] soundwire: intel_init: remove useless test
  2020-05-31 18:20 ` Bard Liao
@ 2020-05-31 18:20   ` Bard Liao
  -1 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:20 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

No need to test link_mask twice

Suggested-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index de0405359db5..e87f17240547 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -109,7 +109,7 @@ static struct sdw_intel_ctx
 
 	/* Create SDW Master devices */
 	for (i = 0; i < count; i++) {
-		if (link_mask && !(link_mask & BIT(i))) {
+		if (!(link_mask & BIT(i))) {
 			dev_dbg(&adev->dev,
 				"Link %d masked, will not be enabled\n", i);
 			link++;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/6] soundwire: intel_init: remove useless test
@ 2020-05-31 18:20   ` Bard Liao
  0 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:20 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: pierre-louis.bossart, vinod.koul, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

No need to test link_mask twice

Suggested-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index de0405359db5..e87f17240547 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -109,7 +109,7 @@ static struct sdw_intel_ctx
 
 	/* Create SDW Master devices */
 	for (i = 0; i < count; i++) {
-		if (link_mask && !(link_mask & BIT(i))) {
+		if (!(link_mask & BIT(i))) {
 			dev_dbg(&adev->dev,
 				"Link %d masked, will not be enabled\n", i);
 			link++;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/6] soundwire: intel_init: use devm_ allocation
  2020-05-31 18:20 ` Bard Liao
@ 2020-05-31 18:21   ` Bard Liao
  -1 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:21 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Make error handling simpler with devm_ allocation.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index e87f17240547..f9d190157c47 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -42,9 +42,6 @@ static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx)
 		link++;
 	}
 
-	kfree(ctx->links);
-	ctx->links = NULL;
-
 	return 0;
 }
 
@@ -96,14 +93,15 @@ static struct sdw_intel_ctx
 
 	dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count);
 
-	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	ctx = devm_kzalloc(&adev->dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return NULL;
 
 	ctx->count = count;
-	ctx->links = kcalloc(ctx->count, sizeof(*ctx->links), GFP_KERNEL);
+	ctx->links = devm_kcalloc(&adev->dev, ctx->count,
+				  sizeof(*ctx->links), GFP_KERNEL);
 	if (!ctx->links)
-		goto link_err;
+		return NULL;
 
 	link = ctx->links;
 
@@ -146,9 +144,8 @@ static struct sdw_intel_ctx
 	return ctx;
 
 pdev_err:
+	ctx->count = i;
 	sdw_intel_cleanup_pdev(ctx);
-link_err:
-	kfree(ctx);
 	return NULL;
 }
 
@@ -216,7 +213,6 @@ void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res)
 void sdw_intel_exit(struct sdw_intel_ctx *ctx)
 {
 	sdw_intel_cleanup_pdev(ctx);
-	kfree(ctx);
 }
 EXPORT_SYMBOL(sdw_intel_exit);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/6] soundwire: intel_init: use devm_ allocation
@ 2020-05-31 18:21   ` Bard Liao
  0 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:21 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: pierre-louis.bossart, vinod.koul, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Make error handling simpler with devm_ allocation.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index e87f17240547..f9d190157c47 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -42,9 +42,6 @@ static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx)
 		link++;
 	}
 
-	kfree(ctx->links);
-	ctx->links = NULL;
-
 	return 0;
 }
 
@@ -96,14 +93,15 @@ static struct sdw_intel_ctx
 
 	dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count);
 
-	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	ctx = devm_kzalloc(&adev->dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return NULL;
 
 	ctx->count = count;
-	ctx->links = kcalloc(ctx->count, sizeof(*ctx->links), GFP_KERNEL);
+	ctx->links = devm_kcalloc(&adev->dev, ctx->count,
+				  sizeof(*ctx->links), GFP_KERNEL);
 	if (!ctx->links)
-		goto link_err;
+		return NULL;
 
 	link = ctx->links;
 
@@ -146,9 +144,8 @@ static struct sdw_intel_ctx
 	return ctx;
 
 pdev_err:
+	ctx->count = i;
 	sdw_intel_cleanup_pdev(ctx);
-link_err:
-	kfree(ctx);
 	return NULL;
 }
 
@@ -216,7 +213,6 @@ void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res)
 void sdw_intel_exit(struct sdw_intel_ctx *ctx)
 {
 	sdw_intel_cleanup_pdev(ctx);
-	kfree(ctx);
 }
 EXPORT_SYMBOL(sdw_intel_exit);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/6] soundwire: intel_init: pass link information as platform data
  2020-05-31 18:20 ` Bard Liao
@ 2020-05-31 18:21   ` Bard Liao
  -1 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:21 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

It's not clear how this code ever worked, the link information is used
in intel.c but never passed as platform_data.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index f9d190157c47..ff1017e64abd 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -128,6 +128,8 @@ static struct sdw_intel_ctx
 		pdevinfo.name = "int-sdw";
 		pdevinfo.id = i;
 		pdevinfo.fwnode = acpi_fwnode_handle(adev);
+		pdevinfo.data = link;
+		pdevinfo.size_data = sizeof(*link);
 
 		pdev = platform_device_register_full(&pdevinfo);
 		if (IS_ERR(pdev)) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/6] soundwire: intel_init: pass link information as platform data
@ 2020-05-31 18:21   ` Bard Liao
  0 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:21 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: pierre-louis.bossart, vinod.koul, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

It's not clear how this code ever worked, the link information is used
in intel.c but never passed as platform_data.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel_init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index f9d190157c47..ff1017e64abd 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -128,6 +128,8 @@ static struct sdw_intel_ctx
 		pdevinfo.name = "int-sdw";
 		pdevinfo.id = i;
 		pdevinfo.fwnode = acpi_fwnode_handle(adev);
+		pdevinfo.data = link;
+		pdevinfo.size_data = sizeof(*link);
 
 		pdev = platform_device_register_full(&pdevinfo);
 		if (IS_ERR(pdev)) {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/6] soundwire: intel: transition to 3 steps initialization
  2020-05-31 18:20 ` Bard Liao
@ 2020-05-31 18:21   ` Bard Liao
  -1 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:21 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Rather than a plain-vanilla init/exit, this patch provides 3 steps in
the initialization needed for driver selection, machine driver
selection and deal with power rail dependencies.

- ACPI scan: this step is done at a very early stage to detect the
presence of a SoundWire Controller and enabled links at the BIOS
level. This step may be called from the legacy HDaudio driver, which
will abort its probe to let the Sound Open Firmware (SOF) handle the
hardware.

- probe: this step allocates all the required memory and will add a
sdw_bus, which in turn will result in identifying all possible Slaves
listed below the Controller ACPI companion device. All the information
is reported to the parent PCI driver which will select the relevant
machine driver.

- startup: this last step starts the bus reset, which results in Slave
devices reporting as ATTACHED and being enumerated. This step is only
done during the card creation stage, after the DSP is powered to
account for internal power rail dependencies.

These 3 steps are already supported in the Sound Open firmware
drivers and upstream.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c      |  60 +++++---
 drivers/soundwire/intel.h      |   2 +
 drivers/soundwire/intel_init.c | 242 ++++++++++++++++++++++++++-------
 3 files changed, 237 insertions(+), 67 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index ef1196ea93a4..f966ac33cd3b 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1074,7 +1074,6 @@ static int intel_init(struct sdw_intel *sdw)
  */
 static int intel_master_probe(struct platform_device *pdev)
 {
-	struct sdw_cdns_stream_config config;
 	struct device *dev = &pdev->dev;
 	struct sdw_intel *sdw;
 	struct sdw_cdns *cdns;
@@ -1112,25 +1111,10 @@ static int intel_master_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (bus->prop.hw_disabled) {
+	if (bus->prop.hw_disabled)
 		dev_info(dev,
 			 "SoundWire master %d is disabled, will be ignored\n",
 			 bus->link_id);
-		return 0;
-	}
-
-	/* Initialize shim, controller and Cadence IP */
-	ret = intel_init(sdw);
-	if (ret)
-		goto err_init;
-
-	/* Read the PDI config and initialize cadence PDI */
-	intel_pdi_init(sdw, &config);
-	ret = sdw_cdns_pdi_init(cdns, config);
-	if (ret)
-		goto err_init;
-
-	intel_pdi_ch_update(sdw);
 
 	/* Acquire IRQ */
 	ret = request_threaded_irq(sdw->link_res->irq,
@@ -1142,6 +1126,42 @@ static int intel_master_probe(struct platform_device *pdev)
 		goto err_init;
 	}
 
+	return 0;
+
+err_init:
+	sdw_bus_master_delete(bus);
+	return ret;
+}
+
+int intel_master_startup(struct platform_device *pdev)
+{
+	struct sdw_cdns_stream_config config;
+	struct device *dev = &pdev->dev;
+	struct sdw_cdns *cdns = dev_get_drvdata(dev);
+	struct sdw_intel *sdw = cdns_to_intel(cdns);
+	struct sdw_bus *bus = &cdns->bus;
+	int ret;
+
+	if (bus->prop.hw_disabled) {
+		dev_info(dev,
+			 "SoundWire master %d is disabled, ignoring\n",
+			 sdw->instance);
+		return 0;
+	}
+
+	/* Initialize shim, controller and Cadence IP */
+	ret = intel_init(sdw);
+	if (ret)
+		goto err_init;
+
+	/* Read the PDI config and initialize cadence PDI */
+	intel_pdi_init(sdw, &config);
+	ret = sdw_cdns_pdi_init(cdns, config);
+	if (ret)
+		goto err_init;
+
+	intel_pdi_ch_update(sdw);
+
 	ret = sdw_cdns_enable_interrupt(cdns, true);
 	if (ret < 0) {
 		dev_err(dev, "cannot enable interrupts\n");
@@ -1168,9 +1188,7 @@ static int intel_master_probe(struct platform_device *pdev)
 
 err_interrupt:
 	sdw_cdns_enable_interrupt(cdns, false);
-	free_irq(sdw->link_res->irq, sdw);
 err_init:
-	sdw_bus_master_delete(bus);
 	return ret;
 }
 
@@ -1196,12 +1214,12 @@ static struct platform_driver sdw_intel_drv = {
 	.probe = intel_master_probe,
 	.remove = intel_master_remove,
 	.driver = {
-		.name = "int-sdw",
+		.name = "intel-sdw",
 	},
 };
 
 module_platform_driver(sdw_intel_drv);
 
 MODULE_LICENSE("Dual BSD/GPL");
-MODULE_ALIAS("platform:int-sdw");
+MODULE_ALIAS("platform:intel-sdw");
 MODULE_DESCRIPTION("Intel Soundwire Master Driver");
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 001968c362cd..1ff71eb4c219 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -38,4 +38,6 @@ struct sdw_intel {
 
 #define SDW_INTEL_QUIRK_MASK_BUS_DISABLE      BIT(1)
 
+int intel_master_startup(struct platform_device *pdev);
+
 #endif /* __SDW_INTEL_LOCAL_H */
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index ff1017e64abd..41c06e9bcecb 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -24,73 +24,146 @@
 #define SDW_LINK_BASE		0x30000
 #define SDW_LINK_SIZE		0x10000
 
-static int link_mask;
-module_param_named(sdw_link_mask, link_mask, int, 0444);
+static int ctrl_link_mask;
+module_param_named(sdw_link_mask, ctrl_link_mask, int, 0444);
 MODULE_PARM_DESC(sdw_link_mask, "Intel link mask (one bit per link)");
 
-static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx)
+static bool is_link_enabled(struct fwnode_handle *fw_node, int i)
+{
+	struct fwnode_handle *link;
+	char name[32];
+	u32 quirk_mask = 0;
+
+	/* Find master handle */
+	snprintf(name, sizeof(name),
+		 "mipi-sdw-link-%d-subproperties", i);
+
+	link = fwnode_get_named_child_node(fw_node, name);
+	if (!link)
+		return false;
+
+	fwnode_property_read_u32(link,
+				 "intel-quirk-mask",
+				 &quirk_mask);
+
+	if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE)
+		return false;
+
+	return true;
+}
+
+static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx)
 {
 	struct sdw_intel_link_res *link = ctx->links;
+	u32 link_mask;
 	int i;
 
 	if (!link)
 		return 0;
 
-	for (i = 0; i < ctx->count; i++) {
+	link_mask = ctx->link_mask;
+
+	for (i = 0; i < ctx->count; i++, link++) {
+		if (!(link_mask & BIT(i)))
+			continue;
+
 		if (link->pdev)
 			platform_device_unregister(link->pdev);
-		link++;
 	}
 
 	return 0;
 }
 
-static struct sdw_intel_ctx
-*sdw_intel_add_controller(struct sdw_intel_res *res)
+static int
+sdw_intel_scan_controller(struct sdw_intel_acpi_info *info)
 {
-	struct platform_device_info pdevinfo;
-	struct platform_device *pdev;
-	struct sdw_intel_link_res *link;
-	struct sdw_intel_ctx *ctx;
 	struct acpi_device *adev;
 	int ret, i;
 	u8 count;
-	u32 caps;
 
-	if (acpi_bus_get_device(res->handle, &adev))
-		return NULL;
+	if (acpi_bus_get_device(info->handle, &adev))
+		return -EINVAL;
 
 	/* Found controller, find links supported */
 	count = 0;
 	ret = fwnode_property_read_u8_array(acpi_fwnode_handle(adev),
 					    "mipi-sdw-master-count", &count, 1);
 
-	/* Don't fail on error, continue and use hw value */
+	/*
+	 * In theory we could check the number of links supported in
+	 * hardware, but in that step we cannot assume SoundWire IP is
+	 * powered.
+	 *
+	 * In addition, if the BIOS doesn't even provide this
+	 * 'master-count' property then all the inits based on link
+	 * masks will fail as well.
+	 *
+	 * We will check the hardware capabilities in the startup() step
+	 */
+
 	if (ret) {
 		dev_err(&adev->dev,
 			"Failed to read mipi-sdw-master-count: %d\n", ret);
-		count = SDW_MAX_LINKS;
+		return -EINVAL;
 	}
 
-	/* Check SNDWLCAP.LCOUNT */
-	caps = ioread32(res->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP);
-	caps &= GENMASK(2, 0);
-
-	/* Check HW supported vs property value and use min of two */
-	count = min_t(u8, caps, count);
-
 	/* Check count is within bounds */
 	if (count > SDW_MAX_LINKS) {
 		dev_err(&adev->dev, "Link count %d exceeds max %d\n",
 			count, SDW_MAX_LINKS);
-		return NULL;
+		return -EINVAL;
 	}
 
 	if (!count) {
 		dev_warn(&adev->dev, "No SoundWire links detected\n");
-		return NULL;
+		return -EINVAL;
 	}
+	dev_dbg(&adev->dev, "ACPI reports %d SDW Link devices\n", count);
+
+	info->count = count;
+	info->link_mask = 0;
+
+	for (i = 0; i < count; i++) {
+		if (ctrl_link_mask && !(ctrl_link_mask & BIT(i))) {
+			dev_dbg(&adev->dev,
+				"Link %d masked, will not be enabled\n", i);
+			continue;
+		}
+
+		if (!is_link_enabled(acpi_fwnode_handle(adev), i)) {
+			dev_dbg(&adev->dev,
+				"Link %d not selected in firmware\n", i);
+			continue;
+		}
+
+		info->link_mask |= BIT(i);
+	}
+
+	return 0;
+}
+
+static struct sdw_intel_ctx
+*sdw_intel_probe_controller(struct sdw_intel_res *res)
+{
+	struct platform_device_info pdevinfo;
+	struct platform_device *pdev;
+	struct sdw_intel_link_res *link;
+	struct sdw_intel_ctx *ctx;
+	struct acpi_device *adev;
+	u32 link_mask;
+	int count;
+	int i;
+
+	if (!res)
+		return NULL;
+
+	if (acpi_bus_get_device(res->handle, &adev))
+		return NULL;
+
+	if (!res->count)
+		return NULL;
 
+	count = res->count;
 	dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count);
 
 	ctx = devm_kzalloc(&adev->dev, sizeof(*ctx), GFP_KERNEL);
@@ -103,19 +176,25 @@ static struct sdw_intel_ctx
 	if (!ctx->links)
 		return NULL;
 
+	ctx->count = count;
+	ctx->mmio_base = res->mmio_base;
+	ctx->link_mask = res->link_mask;
+	ctx->handle = res->handle;
+
 	link = ctx->links;
+	link_mask = ctx->link_mask;
 
 	/* Create SDW Master devices */
-	for (i = 0; i < count; i++) {
+	for (i = 0; i < count; i++, link++) {
 		if (!(link_mask & BIT(i))) {
 			dev_dbg(&adev->dev,
 				"Link %d masked, will not be enabled\n", i);
-			link++;
 			continue;
 		}
 
+		link->mmio_base = res->mmio_base;
 		link->registers = res->mmio_base + SDW_LINK_BASE
-					+ (SDW_LINK_SIZE * i);
+			+ (SDW_LINK_SIZE * i);
 		link->shim = res->mmio_base + SDW_SHIM_BASE;
 		link->alh = res->mmio_base + SDW_ALH_BASE;
 
@@ -125,7 +204,7 @@ static struct sdw_intel_ctx
 		memset(&pdevinfo, 0, sizeof(pdevinfo));
 
 		pdevinfo.parent = res->parent;
-		pdevinfo.name = "int-sdw";
+		pdevinfo.name = "intel-sdw";
 		pdevinfo.id = i;
 		pdevinfo.fwnode = acpi_fwnode_handle(adev);
 		pdevinfo.data = link;
@@ -136,25 +215,63 @@ static struct sdw_intel_ctx
 			dev_err(&adev->dev,
 				"platform device creation failed: %ld\n",
 				PTR_ERR(pdev));
-			goto pdev_err;
+			goto err;
 		}
-
 		link->pdev = pdev;
-		link++;
 	}
 
 	return ctx;
 
-pdev_err:
+err:
 	ctx->count = i;
-	sdw_intel_cleanup_pdev(ctx);
+	sdw_intel_cleanup(ctx);
 	return NULL;
 }
 
+static int
+sdw_intel_startup_controller(struct sdw_intel_ctx *ctx)
+{
+	struct acpi_device *adev;
+	struct sdw_intel_link_res *link;
+	u32 caps;
+	u32 link_mask;
+	int i;
+
+	if (acpi_bus_get_device(ctx->handle, &adev))
+		return -EINVAL;
+
+	/* Check SNDWLCAP.LCOUNT */
+	caps = ioread32(ctx->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP);
+	caps &= GENMASK(2, 0);
+
+	/* Check HW supported vs property value */
+	if (caps < ctx->count) {
+		dev_err(&adev->dev,
+			"BIOS master count is larger than hardware capabilities\n");
+		return -EINVAL;
+	}
+
+	if (!ctx->links)
+		return -EINVAL;
+
+	link = ctx->links;
+	link_mask = ctx->link_mask;
+
+	/* Startup SDW Master devices */
+	for (i = 0; i < ctx->count; i++, link++) {
+		if (!(link_mask & BIT(i)))
+			continue;
+
+		intel_master_startup(link->pdev);
+	}
+
+	return 0;
+}
+
 static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
 				     void *cdata, void **return_value)
 {
-	struct sdw_intel_res *res = cdata;
+	struct sdw_intel_acpi_info *info = cdata;
 	struct acpi_device *adev;
 	acpi_status status;
 	u64 adr;
@@ -168,7 +285,7 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
 		return AE_NOT_FOUND;
 	}
 
-	res->handle = handle;
+	info->handle = handle;
 
 	/*
 	 * On some Intel platforms, multiple children of the HDAS
@@ -185,36 +302,69 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
 }
 
 /**
- * sdw_intel_init() - SoundWire Intel init routine
+ * sdw_intel_acpi_scan() - SoundWire Intel init routine
  * @parent_handle: ACPI parent handle
- * @res: resource data
+ * @info: description of what firmware/DSDT tables expose
  *
- * This scans the namespace and creates SoundWire link controller devices
- * based on the info queried.
+ * This scans the namespace and queries firmware to figure out which
+ * links to enable. A follow-up use of sdw_intel_probe() and
+ * sdw_intel_startup() is required for creation of devices and bus
+ * startup
  */
-void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res)
+int sdw_intel_acpi_scan(acpi_handle *parent_handle,
+			struct sdw_intel_acpi_info *info)
 {
 	acpi_status status;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
 				     parent_handle, 1,
 				     sdw_intel_acpi_cb,
-				     NULL, res, NULL);
+				     NULL, info, NULL);
 	if (ACPI_FAILURE(status))
-		return NULL;
+		return -ENODEV;
 
-	return sdw_intel_add_controller(res);
+	return sdw_intel_scan_controller(info);
 }
+EXPORT_SYMBOL(sdw_intel_acpi_scan);
 
+/**
+ * sdw_intel_probe() - SoundWire Intel probe routine
+ * @res: resource data
+ *
+ * This registers a platform device for each Master handled by the controller,
+ * and SoundWire Master and Slave devices will be created by the platform
+ * device probe. All the information necessary is stored in the context, and
+ * the res argument pointer can be freed after this step.
+ * This function will be called after sdw_intel_acpi_scan() by SOF probe.
+ */
+struct sdw_intel_ctx
+*sdw_intel_probe(struct sdw_intel_res *res)
+{
+	return sdw_intel_probe_controller(res);
+}
+EXPORT_SYMBOL(sdw_intel_probe);
+
+/**
+ * sdw_intel_startup() - SoundWire Intel startup
+ * @ctx: SoundWire context allocated in the probe
+ *
+ * Startup Intel SoundWire controller. This function will be called after
+ * Intel Audio DSP is powered up.
+ */
+int sdw_intel_startup(struct sdw_intel_ctx *ctx)
+{
+	return sdw_intel_startup_controller(ctx);
+}
+EXPORT_SYMBOL(sdw_intel_startup);
 /**
  * sdw_intel_exit() - SoundWire Intel exit
- * @arg: callback context
+ * @ctx: SoundWire context allocated in the probe
  *
  * Delete the controller instances created and cleanup
  */
 void sdw_intel_exit(struct sdw_intel_ctx *ctx)
 {
-	sdw_intel_cleanup_pdev(ctx);
+	sdw_intel_cleanup(ctx);
 }
 EXPORT_SYMBOL(sdw_intel_exit);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/6] soundwire: intel: transition to 3 steps initialization
@ 2020-05-31 18:21   ` Bard Liao
  0 siblings, 0 replies; 16+ messages in thread
From: Bard Liao @ 2020-05-31 18:21 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: pierre-louis.bossart, vinod.koul, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Rather than a plain-vanilla init/exit, this patch provides 3 steps in
the initialization needed for driver selection, machine driver
selection and deal with power rail dependencies.

- ACPI scan: this step is done at a very early stage to detect the
presence of a SoundWire Controller and enabled links at the BIOS
level. This step may be called from the legacy HDaudio driver, which
will abort its probe to let the Sound Open Firmware (SOF) handle the
hardware.

- probe: this step allocates all the required memory and will add a
sdw_bus, which in turn will result in identifying all possible Slaves
listed below the Controller ACPI companion device. All the information
is reported to the parent PCI driver which will select the relevant
machine driver.

- startup: this last step starts the bus reset, which results in Slave
devices reporting as ATTACHED and being enumerated. This step is only
done during the card creation stage, after the DSP is powered to
account for internal power rail dependencies.

These 3 steps are already supported in the Sound Open firmware
drivers and upstream.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/intel.c      |  60 +++++---
 drivers/soundwire/intel.h      |   2 +
 drivers/soundwire/intel_init.c | 242 ++++++++++++++++++++++++++-------
 3 files changed, 237 insertions(+), 67 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index ef1196ea93a4..f966ac33cd3b 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -1074,7 +1074,6 @@ static int intel_init(struct sdw_intel *sdw)
  */
 static int intel_master_probe(struct platform_device *pdev)
 {
-	struct sdw_cdns_stream_config config;
 	struct device *dev = &pdev->dev;
 	struct sdw_intel *sdw;
 	struct sdw_cdns *cdns;
@@ -1112,25 +1111,10 @@ static int intel_master_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	if (bus->prop.hw_disabled) {
+	if (bus->prop.hw_disabled)
 		dev_info(dev,
 			 "SoundWire master %d is disabled, will be ignored\n",
 			 bus->link_id);
-		return 0;
-	}
-
-	/* Initialize shim, controller and Cadence IP */
-	ret = intel_init(sdw);
-	if (ret)
-		goto err_init;
-
-	/* Read the PDI config and initialize cadence PDI */
-	intel_pdi_init(sdw, &config);
-	ret = sdw_cdns_pdi_init(cdns, config);
-	if (ret)
-		goto err_init;
-
-	intel_pdi_ch_update(sdw);
 
 	/* Acquire IRQ */
 	ret = request_threaded_irq(sdw->link_res->irq,
@@ -1142,6 +1126,42 @@ static int intel_master_probe(struct platform_device *pdev)
 		goto err_init;
 	}
 
+	return 0;
+
+err_init:
+	sdw_bus_master_delete(bus);
+	return ret;
+}
+
+int intel_master_startup(struct platform_device *pdev)
+{
+	struct sdw_cdns_stream_config config;
+	struct device *dev = &pdev->dev;
+	struct sdw_cdns *cdns = dev_get_drvdata(dev);
+	struct sdw_intel *sdw = cdns_to_intel(cdns);
+	struct sdw_bus *bus = &cdns->bus;
+	int ret;
+
+	if (bus->prop.hw_disabled) {
+		dev_info(dev,
+			 "SoundWire master %d is disabled, ignoring\n",
+			 sdw->instance);
+		return 0;
+	}
+
+	/* Initialize shim, controller and Cadence IP */
+	ret = intel_init(sdw);
+	if (ret)
+		goto err_init;
+
+	/* Read the PDI config and initialize cadence PDI */
+	intel_pdi_init(sdw, &config);
+	ret = sdw_cdns_pdi_init(cdns, config);
+	if (ret)
+		goto err_init;
+
+	intel_pdi_ch_update(sdw);
+
 	ret = sdw_cdns_enable_interrupt(cdns, true);
 	if (ret < 0) {
 		dev_err(dev, "cannot enable interrupts\n");
@@ -1168,9 +1188,7 @@ static int intel_master_probe(struct platform_device *pdev)
 
 err_interrupt:
 	sdw_cdns_enable_interrupt(cdns, false);
-	free_irq(sdw->link_res->irq, sdw);
 err_init:
-	sdw_bus_master_delete(bus);
 	return ret;
 }
 
@@ -1196,12 +1214,12 @@ static struct platform_driver sdw_intel_drv = {
 	.probe = intel_master_probe,
 	.remove = intel_master_remove,
 	.driver = {
-		.name = "int-sdw",
+		.name = "intel-sdw",
 	},
 };
 
 module_platform_driver(sdw_intel_drv);
 
 MODULE_LICENSE("Dual BSD/GPL");
-MODULE_ALIAS("platform:int-sdw");
+MODULE_ALIAS("platform:intel-sdw");
 MODULE_DESCRIPTION("Intel Soundwire Master Driver");
diff --git a/drivers/soundwire/intel.h b/drivers/soundwire/intel.h
index 001968c362cd..1ff71eb4c219 100644
--- a/drivers/soundwire/intel.h
+++ b/drivers/soundwire/intel.h
@@ -38,4 +38,6 @@ struct sdw_intel {
 
 #define SDW_INTEL_QUIRK_MASK_BUS_DISABLE      BIT(1)
 
+int intel_master_startup(struct platform_device *pdev);
+
 #endif /* __SDW_INTEL_LOCAL_H */
diff --git a/drivers/soundwire/intel_init.c b/drivers/soundwire/intel_init.c
index ff1017e64abd..41c06e9bcecb 100644
--- a/drivers/soundwire/intel_init.c
+++ b/drivers/soundwire/intel_init.c
@@ -24,73 +24,146 @@
 #define SDW_LINK_BASE		0x30000
 #define SDW_LINK_SIZE		0x10000
 
-static int link_mask;
-module_param_named(sdw_link_mask, link_mask, int, 0444);
+static int ctrl_link_mask;
+module_param_named(sdw_link_mask, ctrl_link_mask, int, 0444);
 MODULE_PARM_DESC(sdw_link_mask, "Intel link mask (one bit per link)");
 
-static int sdw_intel_cleanup_pdev(struct sdw_intel_ctx *ctx)
+static bool is_link_enabled(struct fwnode_handle *fw_node, int i)
+{
+	struct fwnode_handle *link;
+	char name[32];
+	u32 quirk_mask = 0;
+
+	/* Find master handle */
+	snprintf(name, sizeof(name),
+		 "mipi-sdw-link-%d-subproperties", i);
+
+	link = fwnode_get_named_child_node(fw_node, name);
+	if (!link)
+		return false;
+
+	fwnode_property_read_u32(link,
+				 "intel-quirk-mask",
+				 &quirk_mask);
+
+	if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE)
+		return false;
+
+	return true;
+}
+
+static int sdw_intel_cleanup(struct sdw_intel_ctx *ctx)
 {
 	struct sdw_intel_link_res *link = ctx->links;
+	u32 link_mask;
 	int i;
 
 	if (!link)
 		return 0;
 
-	for (i = 0; i < ctx->count; i++) {
+	link_mask = ctx->link_mask;
+
+	for (i = 0; i < ctx->count; i++, link++) {
+		if (!(link_mask & BIT(i)))
+			continue;
+
 		if (link->pdev)
 			platform_device_unregister(link->pdev);
-		link++;
 	}
 
 	return 0;
 }
 
-static struct sdw_intel_ctx
-*sdw_intel_add_controller(struct sdw_intel_res *res)
+static int
+sdw_intel_scan_controller(struct sdw_intel_acpi_info *info)
 {
-	struct platform_device_info pdevinfo;
-	struct platform_device *pdev;
-	struct sdw_intel_link_res *link;
-	struct sdw_intel_ctx *ctx;
 	struct acpi_device *adev;
 	int ret, i;
 	u8 count;
-	u32 caps;
 
-	if (acpi_bus_get_device(res->handle, &adev))
-		return NULL;
+	if (acpi_bus_get_device(info->handle, &adev))
+		return -EINVAL;
 
 	/* Found controller, find links supported */
 	count = 0;
 	ret = fwnode_property_read_u8_array(acpi_fwnode_handle(adev),
 					    "mipi-sdw-master-count", &count, 1);
 
-	/* Don't fail on error, continue and use hw value */
+	/*
+	 * In theory we could check the number of links supported in
+	 * hardware, but in that step we cannot assume SoundWire IP is
+	 * powered.
+	 *
+	 * In addition, if the BIOS doesn't even provide this
+	 * 'master-count' property then all the inits based on link
+	 * masks will fail as well.
+	 *
+	 * We will check the hardware capabilities in the startup() step
+	 */
+
 	if (ret) {
 		dev_err(&adev->dev,
 			"Failed to read mipi-sdw-master-count: %d\n", ret);
-		count = SDW_MAX_LINKS;
+		return -EINVAL;
 	}
 
-	/* Check SNDWLCAP.LCOUNT */
-	caps = ioread32(res->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP);
-	caps &= GENMASK(2, 0);
-
-	/* Check HW supported vs property value and use min of two */
-	count = min_t(u8, caps, count);
-
 	/* Check count is within bounds */
 	if (count > SDW_MAX_LINKS) {
 		dev_err(&adev->dev, "Link count %d exceeds max %d\n",
 			count, SDW_MAX_LINKS);
-		return NULL;
+		return -EINVAL;
 	}
 
 	if (!count) {
 		dev_warn(&adev->dev, "No SoundWire links detected\n");
-		return NULL;
+		return -EINVAL;
 	}
+	dev_dbg(&adev->dev, "ACPI reports %d SDW Link devices\n", count);
+
+	info->count = count;
+	info->link_mask = 0;
+
+	for (i = 0; i < count; i++) {
+		if (ctrl_link_mask && !(ctrl_link_mask & BIT(i))) {
+			dev_dbg(&adev->dev,
+				"Link %d masked, will not be enabled\n", i);
+			continue;
+		}
+
+		if (!is_link_enabled(acpi_fwnode_handle(adev), i)) {
+			dev_dbg(&adev->dev,
+				"Link %d not selected in firmware\n", i);
+			continue;
+		}
+
+		info->link_mask |= BIT(i);
+	}
+
+	return 0;
+}
+
+static struct sdw_intel_ctx
+*sdw_intel_probe_controller(struct sdw_intel_res *res)
+{
+	struct platform_device_info pdevinfo;
+	struct platform_device *pdev;
+	struct sdw_intel_link_res *link;
+	struct sdw_intel_ctx *ctx;
+	struct acpi_device *adev;
+	u32 link_mask;
+	int count;
+	int i;
+
+	if (!res)
+		return NULL;
+
+	if (acpi_bus_get_device(res->handle, &adev))
+		return NULL;
+
+	if (!res->count)
+		return NULL;
 
+	count = res->count;
 	dev_dbg(&adev->dev, "Creating %d SDW Link devices\n", count);
 
 	ctx = devm_kzalloc(&adev->dev, sizeof(*ctx), GFP_KERNEL);
@@ -103,19 +176,25 @@ static struct sdw_intel_ctx
 	if (!ctx->links)
 		return NULL;
 
+	ctx->count = count;
+	ctx->mmio_base = res->mmio_base;
+	ctx->link_mask = res->link_mask;
+	ctx->handle = res->handle;
+
 	link = ctx->links;
+	link_mask = ctx->link_mask;
 
 	/* Create SDW Master devices */
-	for (i = 0; i < count; i++) {
+	for (i = 0; i < count; i++, link++) {
 		if (!(link_mask & BIT(i))) {
 			dev_dbg(&adev->dev,
 				"Link %d masked, will not be enabled\n", i);
-			link++;
 			continue;
 		}
 
+		link->mmio_base = res->mmio_base;
 		link->registers = res->mmio_base + SDW_LINK_BASE
-					+ (SDW_LINK_SIZE * i);
+			+ (SDW_LINK_SIZE * i);
 		link->shim = res->mmio_base + SDW_SHIM_BASE;
 		link->alh = res->mmio_base + SDW_ALH_BASE;
 
@@ -125,7 +204,7 @@ static struct sdw_intel_ctx
 		memset(&pdevinfo, 0, sizeof(pdevinfo));
 
 		pdevinfo.parent = res->parent;
-		pdevinfo.name = "int-sdw";
+		pdevinfo.name = "intel-sdw";
 		pdevinfo.id = i;
 		pdevinfo.fwnode = acpi_fwnode_handle(adev);
 		pdevinfo.data = link;
@@ -136,25 +215,63 @@ static struct sdw_intel_ctx
 			dev_err(&adev->dev,
 				"platform device creation failed: %ld\n",
 				PTR_ERR(pdev));
-			goto pdev_err;
+			goto err;
 		}
-
 		link->pdev = pdev;
-		link++;
 	}
 
 	return ctx;
 
-pdev_err:
+err:
 	ctx->count = i;
-	sdw_intel_cleanup_pdev(ctx);
+	sdw_intel_cleanup(ctx);
 	return NULL;
 }
 
+static int
+sdw_intel_startup_controller(struct sdw_intel_ctx *ctx)
+{
+	struct acpi_device *adev;
+	struct sdw_intel_link_res *link;
+	u32 caps;
+	u32 link_mask;
+	int i;
+
+	if (acpi_bus_get_device(ctx->handle, &adev))
+		return -EINVAL;
+
+	/* Check SNDWLCAP.LCOUNT */
+	caps = ioread32(ctx->mmio_base + SDW_SHIM_BASE + SDW_SHIM_LCAP);
+	caps &= GENMASK(2, 0);
+
+	/* Check HW supported vs property value */
+	if (caps < ctx->count) {
+		dev_err(&adev->dev,
+			"BIOS master count is larger than hardware capabilities\n");
+		return -EINVAL;
+	}
+
+	if (!ctx->links)
+		return -EINVAL;
+
+	link = ctx->links;
+	link_mask = ctx->link_mask;
+
+	/* Startup SDW Master devices */
+	for (i = 0; i < ctx->count; i++, link++) {
+		if (!(link_mask & BIT(i)))
+			continue;
+
+		intel_master_startup(link->pdev);
+	}
+
+	return 0;
+}
+
 static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
 				     void *cdata, void **return_value)
 {
-	struct sdw_intel_res *res = cdata;
+	struct sdw_intel_acpi_info *info = cdata;
 	struct acpi_device *adev;
 	acpi_status status;
 	u64 adr;
@@ -168,7 +285,7 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
 		return AE_NOT_FOUND;
 	}
 
-	res->handle = handle;
+	info->handle = handle;
 
 	/*
 	 * On some Intel platforms, multiple children of the HDAS
@@ -185,36 +302,69 @@ static acpi_status sdw_intel_acpi_cb(acpi_handle handle, u32 level,
 }
 
 /**
- * sdw_intel_init() - SoundWire Intel init routine
+ * sdw_intel_acpi_scan() - SoundWire Intel init routine
  * @parent_handle: ACPI parent handle
- * @res: resource data
+ * @info: description of what firmware/DSDT tables expose
  *
- * This scans the namespace and creates SoundWire link controller devices
- * based on the info queried.
+ * This scans the namespace and queries firmware to figure out which
+ * links to enable. A follow-up use of sdw_intel_probe() and
+ * sdw_intel_startup() is required for creation of devices and bus
+ * startup
  */
-void *sdw_intel_init(acpi_handle *parent_handle, struct sdw_intel_res *res)
+int sdw_intel_acpi_scan(acpi_handle *parent_handle,
+			struct sdw_intel_acpi_info *info)
 {
 	acpi_status status;
 
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
 				     parent_handle, 1,
 				     sdw_intel_acpi_cb,
-				     NULL, res, NULL);
+				     NULL, info, NULL);
 	if (ACPI_FAILURE(status))
-		return NULL;
+		return -ENODEV;
 
-	return sdw_intel_add_controller(res);
+	return sdw_intel_scan_controller(info);
 }
+EXPORT_SYMBOL(sdw_intel_acpi_scan);
 
+/**
+ * sdw_intel_probe() - SoundWire Intel probe routine
+ * @res: resource data
+ *
+ * This registers a platform device for each Master handled by the controller,
+ * and SoundWire Master and Slave devices will be created by the platform
+ * device probe. All the information necessary is stored in the context, and
+ * the res argument pointer can be freed after this step.
+ * This function will be called after sdw_intel_acpi_scan() by SOF probe.
+ */
+struct sdw_intel_ctx
+*sdw_intel_probe(struct sdw_intel_res *res)
+{
+	return sdw_intel_probe_controller(res);
+}
+EXPORT_SYMBOL(sdw_intel_probe);
+
+/**
+ * sdw_intel_startup() - SoundWire Intel startup
+ * @ctx: SoundWire context allocated in the probe
+ *
+ * Startup Intel SoundWire controller. This function will be called after
+ * Intel Audio DSP is powered up.
+ */
+int sdw_intel_startup(struct sdw_intel_ctx *ctx)
+{
+	return sdw_intel_startup_controller(ctx);
+}
+EXPORT_SYMBOL(sdw_intel_startup);
 /**
  * sdw_intel_exit() - SoundWire Intel exit
- * @arg: callback context
+ * @ctx: SoundWire context allocated in the probe
  *
  * Delete the controller instances created and cleanup
  */
 void sdw_intel_exit(struct sdw_intel_ctx *ctx)
 {
-	sdw_intel_cleanup_pdev(ctx);
+	sdw_intel_cleanup(ctx);
 }
 EXPORT_SYMBOL(sdw_intel_exit);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/6] soundwire: intel: transition to 3 steps initialization
  2020-05-31 18:20 ` Bard Liao
@ 2020-06-22 11:53   ` Vinod Koul
  -1 siblings, 0 replies; 16+ messages in thread
From: Vinod Koul @ 2020-06-22 11:53 UTC (permalink / raw)
  To: Bard Liao
  Cc: alsa-devel, linux-kernel, tiwai, broonie, gregkh, jank,
	srinivas.kandagatla, rander.wang, ranjani.sridharan, hui.wang,
	pierre-louis.bossart, sanyog.r.kale, slawomir.blauciak,
	mengdong.lin, bard.liao

On 01-06-20, 02:20, Bard Liao wrote:
> This series is to split the original "soundwire: intel: transition to 3
> steps initialization" patch into different patches for better review.
> It also address comments from Vinod.

Applied all, thanks
-- 
~Vinod

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/6] soundwire: intel: transition to 3 steps initialization
@ 2020-06-22 11:53   ` Vinod Koul
  0 siblings, 0 replies; 16+ messages in thread
From: Vinod Koul @ 2020-06-22 11:53 UTC (permalink / raw)
  To: Bard Liao
  Cc: pierre-louis.bossart, alsa-devel, tiwai, gregkh, linux-kernel,
	ranjani.sridharan, hui.wang, broonie, srinivas.kandagatla, jank,
	mengdong.lin, slawomir.blauciak, sanyog.r.kale, rander.wang,
	bard.liao

On 01-06-20, 02:20, Bard Liao wrote:
> This series is to split the original "soundwire: intel: transition to 3
> steps initialization" patch into different patches for better review.
> It also address comments from Vinod.

Applied all, thanks
-- 
~Vinod

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-06-22 11:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-31 18:20 [PATCH v2 0/6] soundwire: intel: transition to 3 steps initialization Bard Liao
2020-05-31 18:20 ` Bard Liao
2020-05-31 18:20 ` [PATCH v2 1/6] soundwire: intel: cleanups for indirections/logs Bard Liao
2020-05-31 18:20   ` Bard Liao
2020-05-31 18:20 ` [PATCH v2 2/6] soundwire: intel: clarify drvdata and remove more indirections Bard Liao
2020-05-31 18:20   ` Bard Liao
2020-05-31 18:20 ` [PATCH v2 3/6] soundwire: intel_init: remove useless test Bard Liao
2020-05-31 18:20   ` Bard Liao
2020-05-31 18:21 ` [PATCH v2 4/6] soundwire: intel_init: use devm_ allocation Bard Liao
2020-05-31 18:21   ` Bard Liao
2020-05-31 18:21 ` [PATCH v2 5/6] soundwire: intel_init: pass link information as platform data Bard Liao
2020-05-31 18:21   ` Bard Liao
2020-05-31 18:21 ` [PATCH v2 6/6] soundwire: intel: transition to 3 steps initialization Bard Liao
2020-05-31 18:21   ` Bard Liao
2020-06-22 11:53 ` [PATCH v2 0/6] " Vinod Koul
2020-06-22 11:53   ` Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.