Skip to content

Commit

Permalink
soundwire: bus: only use CLOCK_STOP_MODE0 and fix confusions
Browse files Browse the repository at this point in the history 
Existing devices and implementations only support the required
CLOCK_STOP_MODE0. All the code related to CLOCK_STOP_MODE1 has not
been tested and is highly questionable, with a clear confusion between
CLOCK_STOP_MODE1 and the simple clock stop state machine.

This patch removes all usages of CLOCK_STOP_MODE1 - which has no
impact on any solution - and fixes the use of the simple clock stop
state machine. The resulting code should be a lot more symmetrical and
easier to maintain.

Note that CLOCK_STOP_MODE1 is not supported in the SoundWire Device
Class specification so it's rather unlikely that we need to re-add
this mode later.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20210511030048.25622-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
  • Loading branch information
Pierre-Louis Bossart authored and Vinod Koul committed May 11, 2021
1 parent 0531e6b commit 345e9f5
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 62 deletions.
100 changes: 40 additions & 60 deletions drivers/soundwire/bus.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -821,26 +821,6 @@ static void sdw_modify_slave_status(struct sdw_slave *slave,
mutex_unlock(&bus->bus_lock);
}

static enum sdw_clk_stop_mode sdw_get_clk_stop_mode(struct sdw_slave *slave)
{
enum sdw_clk_stop_mode mode;

/*
* Query for clock stop mode if Slave implements
* ops->get_clk_stop_mode, else read from property.
*/
if (slave->ops && slave->ops->get_clk_stop_mode) {
mode = slave->ops->get_clk_stop_mode(slave);
} else {
if (slave->prop.clk_stop_mode1)
mode = SDW_CLK_STOP_MODE1;
else
mode = SDW_CLK_STOP_MODE0;
}

return mode;
}

static int sdw_slave_clk_stop_callback(struct sdw_slave *slave,
enum sdw_clk_stop_mode mode,
enum sdw_clk_stop_type type)
Expand Down Expand Up @@ -933,7 +913,6 @@ static int sdw_bus_wait_for_clk_prep_deprep(struct sdw_bus *bus, u16 dev_num)
*/
int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
{
enum sdw_clk_stop_mode slave_mode;
bool simple_clk_stop = true;
struct sdw_slave *slave;
bool is_slave = false;
Expand All @@ -955,33 +934,38 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
/* Identify if Slave(s) are available on Bus */
is_slave = true;

slave_mode = sdw_get_clk_stop_mode(slave);
slave->curr_clk_stop_mode = slave_mode;

ret = sdw_slave_clk_stop_callback(slave, slave_mode,
ret = sdw_slave_clk_stop_callback(slave,
SDW_CLK_STOP_MODE0,
SDW_CLK_PRE_PREPARE);
if (ret < 0) {
dev_err(&slave->dev,
"pre-prepare failed:%d", ret);
return ret;
}

ret = sdw_slave_clk_stop_prepare(slave,
slave_mode, true);
if (ret < 0) {
dev_err(&slave->dev,
"pre-prepare failed:%d", ret);
return ret;
}

if (slave_mode == SDW_CLK_STOP_MODE1)
/* Only prepare a Slave device if needed */
if (!slave->prop.simple_clk_stop_capable) {
simple_clk_stop = false;

ret = sdw_slave_clk_stop_prepare(slave,
SDW_CLK_STOP_MODE0,
true);
if (ret < 0) {
dev_err(&slave->dev,
"pre-prepare failed:%d", ret);
return ret;
}
}
}

/* Skip remaining clock stop preparation if no Slave is attached */
if (!is_slave)
return ret;

/*
* Don't wait for all Slaves to be ready if they follow the simple
* state machine
*/
if (!simple_clk_stop) {
ret = sdw_bus_wait_for_clk_prep_deprep(bus,
SDW_BROADCAST_DEV_NUM);
Expand All @@ -998,17 +982,13 @@ int sdw_bus_prep_clk_stop(struct sdw_bus *bus)
slave->status != SDW_SLAVE_ALERT)
continue;

slave_mode = slave->curr_clk_stop_mode;

if (slave_mode == SDW_CLK_STOP_MODE1) {
ret = sdw_slave_clk_stop_callback(slave,
slave_mode,
SDW_CLK_POST_PREPARE);
ret = sdw_slave_clk_stop_callback(slave,
SDW_CLK_STOP_MODE0,
SDW_CLK_POST_PREPARE);

if (ret < 0) {
dev_err(&slave->dev,
"post-prepare failed:%d", ret);
}
if (ret < 0) {
dev_err(&slave->dev,
"post-prepare failed:%d", ret);
}
}

Expand Down Expand Up @@ -1059,7 +1039,6 @@ EXPORT_SYMBOL(sdw_bus_clk_stop);
*/
int sdw_bus_exit_clk_stop(struct sdw_bus *bus)
{
enum sdw_clk_stop_mode mode;
bool simple_clk_stop = true;
struct sdw_slave *slave;
bool is_slave = false;
Expand All @@ -1081,31 +1060,33 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus)
/* Identify if Slave(s) are available on Bus */
is_slave = true;

mode = slave->curr_clk_stop_mode;

if (mode == SDW_CLK_STOP_MODE1) {
simple_clk_stop = false;
continue;
}

ret = sdw_slave_clk_stop_callback(slave, mode,
ret = sdw_slave_clk_stop_callback(slave, SDW_CLK_STOP_MODE0,
SDW_CLK_PRE_DEPREPARE);
if (ret < 0)
dev_warn(&slave->dev,
"clk stop deprep failed:%d", ret);

ret = sdw_slave_clk_stop_prepare(slave, mode,
false);
/* Only de-prepare a Slave device if needed */
if (!slave->prop.simple_clk_stop_capable) {
simple_clk_stop = false;

if (ret < 0)
dev_warn(&slave->dev,
"clk stop deprep failed:%d", ret);
ret = sdw_slave_clk_stop_prepare(slave, SDW_CLK_STOP_MODE0,
false);

if (ret < 0)
dev_warn(&slave->dev,
"clk stop deprep failed:%d", ret);
}
}

/* Skip remaining clock stop de-preparation if no Slave is attached */
if (!is_slave)
return 0;

/*
* Don't wait for all Slaves to be ready if they follow the simple
* state machine
*/
if (!simple_clk_stop)
sdw_bus_wait_for_clk_prep_deprep(bus, SDW_BROADCAST_DEV_NUM);

Expand All @@ -1117,8 +1098,7 @@ int sdw_bus_exit_clk_stop(struct sdw_bus *bus)
slave->status != SDW_SLAVE_ALERT)
continue;

mode = slave->curr_clk_stop_mode;
sdw_slave_clk_stop_callback(slave, mode,
sdw_slave_clk_stop_callback(slave, SDW_CLK_STOP_MODE0,
SDW_CLK_POST_DEPREPARE);
}

Expand Down
2 changes: 0 additions & 2 deletions include/linux/soundwire/sdw.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,6 @@ struct sdw_slave_ops {
int (*port_prep)(struct sdw_slave *slave,
struct sdw_prepare_ch *prepare_ch,
enum sdw_port_prep_ops pre_ops);
int (*get_clk_stop_mode)(struct sdw_slave *slave);
int (*clk_stop)(struct sdw_slave *slave,
enum sdw_clk_stop_mode mode,
enum sdw_clk_stop_type type);
Expand Down Expand Up @@ -675,7 +674,6 @@ struct sdw_slave {
struct list_head node;
struct completion port_ready[SDW_MAX_PORTS];
unsigned int m_port_map[SDW_MAX_PORTS];
enum sdw_clk_stop_mode curr_clk_stop_mode;
u16 dev_num;
u16 dev_num_sticky;
bool probed;
Expand Down

0 comments on commit 345e9f5

Please sign in to comment.