* [PATCH 4/5] soundwire: qcom: fix error handling in probe
[not found] <20200320162947.17663-1-pierre-louis.bossart@linux.intel.com>
@ 2020-03-20 16:29 ` Pierre-Louis Bossart
2020-03-20 16:29 ` [PATCH 5/5] soundwire: qcom: add sdw_master_device support Pierre-Louis Bossart
1 sibling, 0 replies; 5+ messages in thread
From: Pierre-Louis Bossart @ 2020-03-20 16:29 UTC (permalink / raw)
To: alsa-devel
Cc: linux-kernel, tiwai, broonie, vkoul, gregkh, jank,
srinivas.kandagatla, slawomir.blauciak, Bard liao, Rander Wang,
Ranjani Sridharan, Hui Wang, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, Sanyog Kale, open list:ARM/QUALCOMM SUPPORT
Make sure all error cases are properly handled and all resources freed.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
drivers/soundwire/qcom.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 1c6c6a2e0def..77783ae4b71d 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -756,12 +756,16 @@ static int qcom_swrm_probe(struct platform_device *pdev)
}
ctrl->irq = of_irq_get(dev->of_node, 0);
- if (ctrl->irq < 0)
- return ctrl->irq;
+ if (ctrl->irq < 0) {
+ ret = ctrl->irq;
+ goto err_init;
+ }
ctrl->hclk = devm_clk_get(dev, "iface");
- if (IS_ERR(ctrl->hclk))
- return PTR_ERR(ctrl->hclk);
+ if (IS_ERR(ctrl->hclk)) {
+ ret = PTR_ERR(ctrl->hclk);
+ goto err_init;
+ }
clk_prepare_enable(ctrl->hclk);
@@ -778,7 +782,7 @@ static int qcom_swrm_probe(struct platform_device *pdev)
ret = qcom_swrm_get_port_config(ctrl);
if (ret)
- return ret;
+ goto err_clk;
params = &ctrl->bus.params;
params->max_dr_freq = DEFAULT_CLK_FREQ;
@@ -805,28 +809,32 @@ static int qcom_swrm_probe(struct platform_device *pdev)
"soundwire", ctrl);
if (ret) {
dev_err(dev, "Failed to request soundwire irq\n");
- goto err;
+ goto err_clk;
}
ret = sdw_add_bus_master(&ctrl->bus);
if (ret) {
dev_err(dev, "Failed to register Soundwire controller (%d)\n",
ret);
- goto err;
+ goto err_clk;
}
qcom_swrm_init(ctrl);
ret = qcom_swrm_register_dais(ctrl);
if (ret)
- goto err;
+ goto err_master_add;
dev_info(dev, "Qualcomm Soundwire controller v%x.%x.%x Registered\n",
(ctrl->version >> 24) & 0xff, (ctrl->version >> 16) & 0xff,
ctrl->version & 0xffff);
return 0;
-err:
+
+err_master_add:
+ sdw_delete_bus_master(&ctrl->bus);
+err_clk:
clk_disable_unprepare(ctrl->hclk);
+err_init:
return ret;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 5/5] soundwire: qcom: add sdw_master_device support
[not found] <20200320162947.17663-1-pierre-louis.bossart@linux.intel.com>
2020-03-20 16:29 ` [PATCH 4/5] soundwire: qcom: fix error handling in probe Pierre-Louis Bossart
@ 2020-03-20 16:29 ` Pierre-Louis Bossart
2020-03-20 17:01 ` Srinivas Kandagatla
1 sibling, 1 reply; 5+ messages in thread
From: Pierre-Louis Bossart @ 2020-03-20 16:29 UTC (permalink / raw)
To: alsa-devel
Cc: linux-kernel, tiwai, broonie, vkoul, gregkh, jank,
srinivas.kandagatla, slawomir.blauciak, Bard liao, Rander Wang,
Ranjani Sridharan, Hui Wang, Pierre-Louis Bossart, Andy Gross,
Bjorn Andersson, Sanyog Kale, open list:ARM/QUALCOMM SUPPORT
Add new device as a child of the platform device, following the
following hierarchy:
platform_device
sdw_master_device
sdw_slave0
...
sdw_slaveN
For the Qualcomm implementation no sdw_master_driver is registered so
the dais have to be registered using the platform_device and likely
all power management is handled at the platform device level.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
drivers/soundwire/qcom.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 77783ae4b71d..86b46415e50b 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -89,6 +89,7 @@ struct qcom_swrm_port_config {
struct qcom_swrm_ctrl {
struct sdw_bus bus;
struct device *dev;
+ struct sdw_master_device *md;
struct regmap *regmap;
struct completion *comp;
struct work_struct slave_work;
@@ -775,14 +776,31 @@ static int qcom_swrm_probe(struct platform_device *pdev)
mutex_init(&ctrl->port_lock);
INIT_WORK(&ctrl->slave_work, qcom_swrm_slave_wq);
- ctrl->bus.dev = dev;
+ /*
+ * add sdw_master_device.
+ * For the Qualcomm implementation there is no driver.
+ */
+ ctrl->md = sdw_master_device_add(NULL, /* no driver name */
+ dev, /* platform device is parent */
+ dev->fwnode,
+ 0, /* only one link supported */
+ NULL); /* no context */
+ if (IS_ERR(ctrl->md)) {
+ dev_err(dev, "Could not create sdw_master_device\n");
+ ret = PTR_ERR(ctrl->md);
+ goto err_clk;
+ }
+
+ /* the bus uses the sdw_master_device, not the platform device */
+ ctrl->bus.dev = &ctrl->md->dev;
+
ctrl->bus.ops = &qcom_swrm_ops;
ctrl->bus.port_ops = &qcom_swrm_port_ops;
ctrl->bus.compute_params = &qcom_swrm_compute_params;
ret = qcom_swrm_get_port_config(ctrl);
if (ret)
- goto err_clk;
+ goto err_md;
params = &ctrl->bus.params;
params->max_dr_freq = DEFAULT_CLK_FREQ;
@@ -809,14 +827,14 @@ static int qcom_swrm_probe(struct platform_device *pdev)
"soundwire", ctrl);
if (ret) {
dev_err(dev, "Failed to request soundwire irq\n");
- goto err_clk;
+ goto err_md;
}
ret = sdw_add_bus_master(&ctrl->bus);
if (ret) {
dev_err(dev, "Failed to register Soundwire controller (%d)\n",
ret);
- goto err_clk;
+ goto err_md;
}
qcom_swrm_init(ctrl);
@@ -832,6 +850,8 @@ static int qcom_swrm_probe(struct platform_device *pdev)
err_master_add:
sdw_delete_bus_master(&ctrl->bus);
+err_md:
+ device_unregister(&ctrl->md->dev);
err_clk:
clk_disable_unprepare(ctrl->hclk);
err_init:
@@ -843,6 +863,7 @@ static int qcom_swrm_remove(struct platform_device *pdev)
struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(&pdev->dev);
sdw_delete_bus_master(&ctrl->bus);
+ device_unregister(&ctrl->md->dev);
clk_disable_unprepare(ctrl->hclk);
return 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 5/5] soundwire: qcom: add sdw_master_device support
2020-03-20 16:29 ` [PATCH 5/5] soundwire: qcom: add sdw_master_device support Pierre-Louis Bossart
@ 2020-03-20 17:01 ` Srinivas Kandagatla
2020-03-20 17:57 ` Pierre-Louis Bossart
2020-03-23 12:52 ` Vinod Koul
0 siblings, 2 replies; 5+ messages in thread
From: Srinivas Kandagatla @ 2020-03-20 17:01 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel
Cc: linux-kernel, tiwai, broonie, vkoul, gregkh, jank,
slawomir.blauciak, Bard liao, Rander Wang, Ranjani Sridharan,
Hui Wang, Andy Gross, Bjorn Andersson, Sanyog Kale,
open list:ARM/QUALCOMM SUPPORT
On 20/03/2020 16:29, Pierre-Louis Bossart wrote:
> Add new device as a child of the platform device, following the
> following hierarchy:
>
> platform_device
> sdw_master_device
> sdw_slave0
Why can't we just remove the platform device layer here and add
sdw_master_device directly?
What is it stopping doing that?
--srini
> ...
> sdw_slaveN
>
> For the Qualcomm implementation no sdw_master_driver is registered so
> the dais have to be registered using the platform_device and likely
> all power management is handled at the platform device level.
>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
> drivers/soundwire/qcom.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 77783ae4b71d..86b46415e50b 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -89,6 +89,7 @@ struct qcom_swrm_port_config {
> struct qcom_swrm_ctrl {
> struct sdw_bus bus;
> struct device *dev;
> + struct sdw_master_device *md;
> struct regmap *regmap;
> struct completion *comp;
> struct work_struct slave_work;
> @@ -775,14 +776,31 @@ static int qcom_swrm_probe(struct platform_device *pdev)
> mutex_init(&ctrl->port_lock);
> INIT_WORK(&ctrl->slave_work, qcom_swrm_slave_wq);
>
> - ctrl->bus.dev = dev;
> + /*
> + * add sdw_master_device.
> + * For the Qualcomm implementation there is no driver.
> + */
> + ctrl->md = sdw_master_device_add(NULL, /* no driver name */
> + dev, /* platform device is parent */
> + dev->fwnode,
> + 0, /* only one link supported */
> + NULL); /* no context */
> + if (IS_ERR(ctrl->md)) {
> + dev_err(dev, "Could not create sdw_master_device\n");
> + ret = PTR_ERR(ctrl->md);
> + goto err_clk;
> + }
> +
> + /* the bus uses the sdw_master_device, not the platform device */
> + ctrl->bus.dev = &ctrl->md->dev;
> +
> ctrl->bus.ops = &qcom_swrm_ops;
> ctrl->bus.port_ops = &qcom_swrm_port_ops;
> ctrl->bus.compute_params = &qcom_swrm_compute_params;
>
> ret = qcom_swrm_get_port_config(ctrl);
> if (ret)
> - goto err_clk;
> + goto err_md;
>
> params = &ctrl->bus.params;
> params->max_dr_freq = DEFAULT_CLK_FREQ;
> @@ -809,14 +827,14 @@ static int qcom_swrm_probe(struct platform_device *pdev)
> "soundwire", ctrl);
> if (ret) {
> dev_err(dev, "Failed to request soundwire irq\n");
> - goto err_clk;
> + goto err_md;
> }
>
> ret = sdw_add_bus_master(&ctrl->bus);
> if (ret) {
> dev_err(dev, "Failed to register Soundwire controller (%d)\n",
> ret);
> - goto err_clk;
> + goto err_md;
> }
>
> qcom_swrm_init(ctrl);
> @@ -832,6 +850,8 @@ static int qcom_swrm_probe(struct platform_device *pdev)
>
> err_master_add:
> sdw_delete_bus_master(&ctrl->bus);
> +err_md:
> + device_unregister(&ctrl->md->dev);
> err_clk:
> clk_disable_unprepare(ctrl->hclk);
> err_init:
> @@ -843,6 +863,7 @@ static int qcom_swrm_remove(struct platform_device *pdev)
> struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(&pdev->dev);
>
> sdw_delete_bus_master(&ctrl->bus);
> + device_unregister(&ctrl->md->dev);
> clk_disable_unprepare(ctrl->hclk);
>
> return 0;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/5] soundwire: qcom: add sdw_master_device support
2020-03-20 17:01 ` Srinivas Kandagatla
@ 2020-03-20 17:57 ` Pierre-Louis Bossart
2020-03-23 12:52 ` Vinod Koul
1 sibling, 0 replies; 5+ messages in thread
From: Pierre-Louis Bossart @ 2020-03-20 17:57 UTC (permalink / raw)
To: Srinivas Kandagatla, alsa-devel
Cc: tiwai, gregkh, linux-kernel, Ranjani Sridharan, Hui Wang, vkoul,
broonie, Andy Gross, jank, open list:ARM/QUALCOMM SUPPORT,
slawomir.blauciak, Sanyog Kale, Bjorn Andersson, Bard liao,
Rander Wang
>> Add new device as a child of the platform device, following the
>> following hierarchy:
>>
>> platform_device
>> sdw_master_device
>> sdw_slave0
>
> Why can't we just remove the platform device layer here and add
> sdw_master_device directly?
>
> What is it stopping doing that?
The guidance from Greg was "no platform devices, unless you really are
on a platform bus (i.e. Device tree.)". We never discussed changing the
way the Device Tree parts are handled.
The main idea was to leave the parent (be it platform-device or PCI
device) alone and not add new attributes or references to it.
The scheme here is similar to I2C/SPI, you have a platform device
handled by the Device Tree baseline, and a driver create an
i2c_adapter/spi_controller/sdw_master_device.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 5/5] soundwire: qcom: add sdw_master_device support
2020-03-20 17:01 ` Srinivas Kandagatla
2020-03-20 17:57 ` Pierre-Louis Bossart
@ 2020-03-23 12:52 ` Vinod Koul
1 sibling, 0 replies; 5+ messages in thread
From: Vinod Koul @ 2020-03-23 12:52 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Pierre-Louis Bossart, alsa-devel, linux-kernel, tiwai, broonie,
gregkh, jank, slawomir.blauciak, Bard liao, Rander Wang,
Ranjani Sridharan, Hui Wang, Andy Gross, Bjorn Andersson,
Sanyog Kale, open list:ARM/QUALCOMM SUPPORT
On 20-03-20, 17:01, Srinivas Kandagatla wrote:
>
>
> On 20/03/2020 16:29, Pierre-Louis Bossart wrote:
> > Add new device as a child of the platform device, following the
> > following hierarchy:
> >
> > platform_device
> > sdw_master_device
> > sdw_slave0
>
> Why can't we just remove the platform device layer here and add
> sdw_master_device directly?
In the case platform_device is the OF device your controller gets probed
on.
My thinking on this is that drivers should not be directly creating
sdw_master_device but it should be done by core as this device is for
core to use and handle. Ideally I would love that sdw_master_device is
created/handled by core, preferably this be handled as part of
sdw_add_bus_master().
But Pierre is trying to solve the limitation of the devices given by
ACPI and trying to add sdw_master_driver to handle that. I am not
convinced that we should do that.
--
~Vinod
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-23 12:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200320162947.17663-1-pierre-louis.bossart@linux.intel.com>
2020-03-20 16:29 ` [PATCH 4/5] soundwire: qcom: fix error handling in probe Pierre-Louis Bossart
2020-03-20 16:29 ` [PATCH 5/5] soundwire: qcom: add sdw_master_device support Pierre-Louis Bossart
2020-03-20 17:01 ` Srinivas Kandagatla
2020-03-20 17:57 ` Pierre-Louis Bossart
2020-03-23 12:52 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).