From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: <vkoul@kernel.org>
Cc: alsa-devel@alsa-project.org, patches@opensource.cirrus.com,
pierre-louis.bossart@linux.intel.com,
linux-kernel@vger.kernel.org, sanyog.r.kale@intel.com,
yung-chuan.liao@linux.intel.com
Subject: [PATCH v4 4/4] soundwire: stream: Move remaining register accesses over to no_pm
Date: Fri, 25 Nov 2022 14:20:28 +0000 [thread overview]
Message-ID: <20221125142028.1118618-5-ckeepax@opensource.cirrus.com> (raw)
In-Reply-To: <20221125142028.1118618-1-ckeepax@opensource.cirrus.com>
There is no need to play with the runtime reference everytime a register
is accessed. All the remaining "pm" style register accesses trace back
to 4 functions:
sdw_prepare_stream
sdw_deprepare_stream
sdw_enable_stream
sdw_disable_stream
Any sensible implementation will need to hold a runtime reference
across all those functions, it makes no sense to be allowing the
device/bus to suspend whilst streams are being prepared/enabled. And
certainly in the case of the all existing users, they all call these
functions from hw_params/prepare/trigger/hw_free callbacks in ALSA,
which will have already runtime resumed all the audio devices
associated during the open callback.
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No change since v3.
drivers/soundwire/bus.c | 2 +-
drivers/soundwire/stream.c | 30 +++++++++++++++---------------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index ef4878258afad..d87a188fcce1e 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1214,7 +1214,7 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave,
val &= ~SDW_DPN_INT_PORT_READY;
}
- ret = sdw_update(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
+ ret = sdw_update_no_pm(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
if (ret < 0)
dev_err(&slave->dev,
"SDW_DPN_INTMASK write failed:%d\n", val);
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index bd502368339e5..df3b36670df4c 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -81,14 +81,14 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
}
/* Program DPN_OffsetCtrl2 registers */
- ret = sdw_write(slave, addr1, t_params->offset2);
+ ret = sdw_write_no_pm(slave, addr1, t_params->offset2);
if (ret < 0) {
dev_err(bus->dev, "DPN_OffsetCtrl2 register write failed\n");
return ret;
}
/* Program DPN_BlockCtrl3 register */
- ret = sdw_write(slave, addr2, t_params->blk_pkg_mode);
+ ret = sdw_write_no_pm(slave, addr2, t_params->blk_pkg_mode);
if (ret < 0) {
dev_err(bus->dev, "DPN_BlockCtrl3 register write failed\n");
return ret;
@@ -105,7 +105,7 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
/* Program DPN_SampleCtrl2 register */
wbuf = FIELD_GET(SDW_DPN_SAMPLECTRL_HIGH, t_params->sample_interval - 1);
- ret = sdw_write(slave, addr3, wbuf);
+ ret = sdw_write_no_pm(slave, addr3, wbuf);
if (ret < 0) {
dev_err(bus->dev, "DPN_SampleCtrl2 register write failed\n");
return ret;
@@ -115,7 +115,7 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
wbuf = FIELD_PREP(SDW_DPN_HCTRL_HSTART, t_params->hstart);
wbuf |= FIELD_PREP(SDW_DPN_HCTRL_HSTOP, t_params->hstop);
- ret = sdw_write(slave, addr4, wbuf);
+ ret = sdw_write_no_pm(slave, addr4, wbuf);
if (ret < 0)
dev_err(bus->dev, "DPN_HCtrl register write failed\n");
@@ -163,7 +163,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
wbuf = FIELD_PREP(SDW_DPN_PORTCTRL_DATAMODE, p_params->data_mode);
wbuf |= FIELD_PREP(SDW_DPN_PORTCTRL_FLOWMODE, p_params->flow_mode);
- ret = sdw_update(s_rt->slave, addr1, 0xF, wbuf);
+ ret = sdw_update_no_pm(s_rt->slave, addr1, 0xF, wbuf);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_PortCtrl register write failed for port %d\n",
@@ -173,7 +173,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
if (!dpn_prop->read_only_wordlength) {
/* Program DPN_BlockCtrl1 register */
- ret = sdw_write(s_rt->slave, addr2, (p_params->bps - 1));
+ ret = sdw_write_no_pm(s_rt->slave, addr2, (p_params->bps - 1));
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_BlockCtrl1 register write failed for port %d\n",
@@ -184,7 +184,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
/* Program DPN_SampleCtrl1 register */
wbuf = (t_params->sample_interval - 1) & SDW_DPN_SAMPLECTRL_LOW;
- ret = sdw_write(s_rt->slave, addr3, wbuf);
+ ret = sdw_write_no_pm(s_rt->slave, addr3, wbuf);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_SampleCtrl1 register write failed for port %d\n",
@@ -193,7 +193,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
}
/* Program DPN_OffsetCtrl1 registers */
- ret = sdw_write(s_rt->slave, addr4, t_params->offset1);
+ ret = sdw_write_no_pm(s_rt->slave, addr4, t_params->offset1);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_OffsetCtrl1 register write failed for port %d\n",
@@ -203,7 +203,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
/* Program DPN_BlockCtrl2 register*/
if (t_params->blk_grp_ctrl_valid) {
- ret = sdw_write(s_rt->slave, addr5, t_params->blk_grp_ctrl);
+ ret = sdw_write_no_pm(s_rt->slave, addr5, t_params->blk_grp_ctrl);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_BlockCtrl2 reg write failed for port %d\n",
@@ -214,7 +214,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
/* program DPN_LaneCtrl register */
if (slave_prop->lane_control_support) {
- ret = sdw_write(s_rt->slave, addr6, t_params->lane_ctrl);
+ ret = sdw_write_no_pm(s_rt->slave, addr6, t_params->lane_ctrl);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_LaneCtrl register write failed for port %d\n",
@@ -319,9 +319,9 @@ static int sdw_enable_disable_slave_ports(struct sdw_bus *bus,
* it is safe to reset this register
*/
if (en)
- ret = sdw_write(s_rt->slave, addr, p_rt->ch_mask);
+ ret = sdw_write_no_pm(s_rt->slave, addr, p_rt->ch_mask);
else
- ret = sdw_write(s_rt->slave, addr, 0x0);
+ ret = sdw_write_no_pm(s_rt->slave, addr, 0x0);
if (ret < 0)
dev_err(&s_rt->slave->dev,
@@ -476,9 +476,9 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
addr = SDW_DPN_PREPARECTRL(p_rt->num);
if (prep)
- ret = sdw_write(s_rt->slave, addr, p_rt->ch_mask);
+ ret = sdw_write_no_pm(s_rt->slave, addr, p_rt->ch_mask);
else
- ret = sdw_write(s_rt->slave, addr, 0x0);
+ ret = sdw_write_no_pm(s_rt->slave, addr, 0x0);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
@@ -491,7 +491,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
wait_for_completion_timeout(port_ready,
msecs_to_jiffies(dpn_prop->ch_prep_timeout));
- val = sdw_read(s_rt->slave, SDW_DPN_PREPARESTATUS(p_rt->num));
+ val = sdw_read_no_pm(s_rt->slave, SDW_DPN_PREPARESTATUS(p_rt->num));
if ((val < 0) || (val & p_rt->ch_mask)) {
ret = (val < 0) ? val : -ETIMEDOUT;
dev_err(&s_rt->slave->dev,
--
2.30.2
WARNING: multiple messages have this Message-ID (diff)
From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: <vkoul@kernel.org>
Cc: <yung-chuan.liao@linux.intel.com>,
<pierre-louis.bossart@linux.intel.com>, <sanyog.r.kale@intel.com>,
<alsa-devel@alsa-project.org>, <linux-kernel@vger.kernel.org>,
<patches@opensource.cirrus.com>
Subject: [PATCH v4 4/4] soundwire: stream: Move remaining register accesses over to no_pm
Date: Fri, 25 Nov 2022 14:20:28 +0000 [thread overview]
Message-ID: <20221125142028.1118618-5-ckeepax@opensource.cirrus.com> (raw)
In-Reply-To: <20221125142028.1118618-1-ckeepax@opensource.cirrus.com>
There is no need to play with the runtime reference everytime a register
is accessed. All the remaining "pm" style register accesses trace back
to 4 functions:
sdw_prepare_stream
sdw_deprepare_stream
sdw_enable_stream
sdw_disable_stream
Any sensible implementation will need to hold a runtime reference
across all those functions, it makes no sense to be allowing the
device/bus to suspend whilst streams are being prepared/enabled. And
certainly in the case of the all existing users, they all call these
functions from hw_params/prepare/trigger/hw_free callbacks in ALSA,
which will have already runtime resumed all the audio devices
associated during the open callback.
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
No change since v3.
drivers/soundwire/bus.c | 2 +-
drivers/soundwire/stream.c | 30 +++++++++++++++---------------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index ef4878258afad..d87a188fcce1e 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1214,7 +1214,7 @@ int sdw_configure_dpn_intr(struct sdw_slave *slave,
val &= ~SDW_DPN_INT_PORT_READY;
}
- ret = sdw_update(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
+ ret = sdw_update_no_pm(slave, addr, (mask | SDW_DPN_INT_PORT_READY), val);
if (ret < 0)
dev_err(&slave->dev,
"SDW_DPN_INTMASK write failed:%d\n", val);
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index bd502368339e5..df3b36670df4c 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -81,14 +81,14 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
}
/* Program DPN_OffsetCtrl2 registers */
- ret = sdw_write(slave, addr1, t_params->offset2);
+ ret = sdw_write_no_pm(slave, addr1, t_params->offset2);
if (ret < 0) {
dev_err(bus->dev, "DPN_OffsetCtrl2 register write failed\n");
return ret;
}
/* Program DPN_BlockCtrl3 register */
- ret = sdw_write(slave, addr2, t_params->blk_pkg_mode);
+ ret = sdw_write_no_pm(slave, addr2, t_params->blk_pkg_mode);
if (ret < 0) {
dev_err(bus->dev, "DPN_BlockCtrl3 register write failed\n");
return ret;
@@ -105,7 +105,7 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
/* Program DPN_SampleCtrl2 register */
wbuf = FIELD_GET(SDW_DPN_SAMPLECTRL_HIGH, t_params->sample_interval - 1);
- ret = sdw_write(slave, addr3, wbuf);
+ ret = sdw_write_no_pm(slave, addr3, wbuf);
if (ret < 0) {
dev_err(bus->dev, "DPN_SampleCtrl2 register write failed\n");
return ret;
@@ -115,7 +115,7 @@ static int _sdw_program_slave_port_params(struct sdw_bus *bus,
wbuf = FIELD_PREP(SDW_DPN_HCTRL_HSTART, t_params->hstart);
wbuf |= FIELD_PREP(SDW_DPN_HCTRL_HSTOP, t_params->hstop);
- ret = sdw_write(slave, addr4, wbuf);
+ ret = sdw_write_no_pm(slave, addr4, wbuf);
if (ret < 0)
dev_err(bus->dev, "DPN_HCtrl register write failed\n");
@@ -163,7 +163,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
wbuf = FIELD_PREP(SDW_DPN_PORTCTRL_DATAMODE, p_params->data_mode);
wbuf |= FIELD_PREP(SDW_DPN_PORTCTRL_FLOWMODE, p_params->flow_mode);
- ret = sdw_update(s_rt->slave, addr1, 0xF, wbuf);
+ ret = sdw_update_no_pm(s_rt->slave, addr1, 0xF, wbuf);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_PortCtrl register write failed for port %d\n",
@@ -173,7 +173,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
if (!dpn_prop->read_only_wordlength) {
/* Program DPN_BlockCtrl1 register */
- ret = sdw_write(s_rt->slave, addr2, (p_params->bps - 1));
+ ret = sdw_write_no_pm(s_rt->slave, addr2, (p_params->bps - 1));
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_BlockCtrl1 register write failed for port %d\n",
@@ -184,7 +184,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
/* Program DPN_SampleCtrl1 register */
wbuf = (t_params->sample_interval - 1) & SDW_DPN_SAMPLECTRL_LOW;
- ret = sdw_write(s_rt->slave, addr3, wbuf);
+ ret = sdw_write_no_pm(s_rt->slave, addr3, wbuf);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_SampleCtrl1 register write failed for port %d\n",
@@ -193,7 +193,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
}
/* Program DPN_OffsetCtrl1 registers */
- ret = sdw_write(s_rt->slave, addr4, t_params->offset1);
+ ret = sdw_write_no_pm(s_rt->slave, addr4, t_params->offset1);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_OffsetCtrl1 register write failed for port %d\n",
@@ -203,7 +203,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
/* Program DPN_BlockCtrl2 register*/
if (t_params->blk_grp_ctrl_valid) {
- ret = sdw_write(s_rt->slave, addr5, t_params->blk_grp_ctrl);
+ ret = sdw_write_no_pm(s_rt->slave, addr5, t_params->blk_grp_ctrl);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_BlockCtrl2 reg write failed for port %d\n",
@@ -214,7 +214,7 @@ static int sdw_program_slave_port_params(struct sdw_bus *bus,
/* program DPN_LaneCtrl register */
if (slave_prop->lane_control_support) {
- ret = sdw_write(s_rt->slave, addr6, t_params->lane_ctrl);
+ ret = sdw_write_no_pm(s_rt->slave, addr6, t_params->lane_ctrl);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
"DPN_LaneCtrl register write failed for port %d\n",
@@ -319,9 +319,9 @@ static int sdw_enable_disable_slave_ports(struct sdw_bus *bus,
* it is safe to reset this register
*/
if (en)
- ret = sdw_write(s_rt->slave, addr, p_rt->ch_mask);
+ ret = sdw_write_no_pm(s_rt->slave, addr, p_rt->ch_mask);
else
- ret = sdw_write(s_rt->slave, addr, 0x0);
+ ret = sdw_write_no_pm(s_rt->slave, addr, 0x0);
if (ret < 0)
dev_err(&s_rt->slave->dev,
@@ -476,9 +476,9 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
addr = SDW_DPN_PREPARECTRL(p_rt->num);
if (prep)
- ret = sdw_write(s_rt->slave, addr, p_rt->ch_mask);
+ ret = sdw_write_no_pm(s_rt->slave, addr, p_rt->ch_mask);
else
- ret = sdw_write(s_rt->slave, addr, 0x0);
+ ret = sdw_write_no_pm(s_rt->slave, addr, 0x0);
if (ret < 0) {
dev_err(&s_rt->slave->dev,
@@ -491,7 +491,7 @@ static int sdw_prep_deprep_slave_ports(struct sdw_bus *bus,
wait_for_completion_timeout(port_ready,
msecs_to_jiffies(dpn_prop->ch_prep_timeout));
- val = sdw_read(s_rt->slave, SDW_DPN_PREPARESTATUS(p_rt->num));
+ val = sdw_read_no_pm(s_rt->slave, SDW_DPN_PREPARESTATUS(p_rt->num));
if ((val < 0) || (val & p_rt->ch_mask)) {
ret = (val < 0) ? val : -ETIMEDOUT;
dev_err(&s_rt->slave->dev,
--
2.30.2
next prev parent reply other threads:[~2022-11-25 14:21 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-25 14:20 [PATCH v4 0/4] Minor SoundWire clean ups Charles Keepax
2022-11-25 14:20 ` Charles Keepax
2022-11-25 14:20 ` [PATCH v4 1/4] soundwire: bus: export sdw_nwrite_no_pm and sdw_nread_no_pm functions Charles Keepax
2022-11-25 14:20 ` Charles Keepax
2022-11-25 14:20 ` [PATCH v4 2/4] soundwire: Provide build stubs for common functions Charles Keepax
2022-11-25 14:20 ` Charles Keepax
2022-11-25 14:20 ` [PATCH v4 3/4] soundwire: debugfs: Switch to sdw_read_no_pm Charles Keepax
2022-11-25 14:20 ` Charles Keepax
2022-11-25 14:20 ` Charles Keepax [this message]
2022-11-25 14:20 ` [PATCH v4 4/4] soundwire: stream: Move remaining register accesses over to no_pm Charles Keepax
2023-01-09 16:16 ` [PATCH v4 0/4] Minor SoundWire clean ups Vinod Koul
2023-01-09 16:16 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221125142028.1118618-5-ckeepax@opensource.cirrus.com \
--to=ckeepax@opensource.cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=linux-kernel@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=sanyog.r.kale@intel.com \
--cc=vkoul@kernel.org \
--cc=yung-chuan.liao@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.