Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sdw rework #3

Closed
wants to merge 4 commits into from
Closed

Sdw rework #3

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
343e424
ASoC: codec: rt700: Add prepare support in rt700
RanderWang Aug 13, 2019
835b6a8
ASoC: SOF: sdw: revert change for sdw dai in sof driver
RanderWang Aug 13, 2019
7c0809b
soundwire: intel: add more features for sdw dai driver
RanderWang Aug 13, 2019
847e2a0
ASoC: SOF: sdw: add a callback function to free resource in FW
RanderWang Aug 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 39 additions & 25 deletions sound/soc/codecs/rt700.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1232,20 +1232,14 @@ static int rt700_pcm_hw_params(struct snd_pcm_substream *substream,
{
struct snd_soc_component *component = dai->component;
struct rt700_priv *rt700 = snd_soc_component_get_drvdata(component);
struct sdw_stream_config stream_config;
struct sdw_port_config port_config;
struct sdw_stream_config *stream_config;
struct sdw_port_config *port_config;
enum sdw_data_direction direction;
struct sdw_stream_data *stream;
int retval, port, num_channels;
int port, num_channels;
unsigned int val = 0;

dev_err(dai->dev, "%s %s", __func__, dai->name);
stream = snd_soc_dai_get_dma_data(dai, substream);

if (!stream)
return -ENOMEM;

dev_err(dai->dev, "1 %s %s", __func__, dai->name);
if (!rt700->slave)
return 0;

Expand All @@ -1269,24 +1263,17 @@ static int rt700_pcm_hw_params(struct snd_pcm_substream *substream,
dev_err(component->dev, "Invalid DAI id %d\n", dai->id);
return -EINVAL;
}
dev_err(dai->dev, "2 %s %s", __func__, dai->name);

stream_config.frame_rate = params_rate(params);
stream_config.ch_count = params_channels(params);
stream_config.bps = snd_pcm_format_width(params_format(params));
stream_config.direction = direction;
stream_config = &rt700->stream_config;
stream_config->frame_rate = params_rate(params);
stream_config->ch_count = params_channels(params);
stream_config->bps = snd_pcm_format_width(params_format(params));
stream_config->direction = direction;

dev_err(dai->dev, "3 %s %s", __func__, dai->name);
num_channels = params_channels(params);
port_config.ch_mask = (1 << (num_channels)) - 1;
port_config.num = port;

retval = sdw_stream_add_slave(rt700->slave, &stream_config,
&port_config, 1, stream->sdw_stream);
if (retval) {
dev_err(dai->dev, "Unable to configure port\n");
return retval;
}
port_config = &rt700->port_config;
port_config->ch_mask = (1 << (num_channels)) - 1;
port_config->num = port;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

port is not initialized?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why you need those two structures? Is this to memorize information that will be used in prepare? If yes, maybe you can add comments to help the reviewer, there are none in your changes...


dev_err(dai->dev, "4 %s %s", __func__, dai->name);
switch (params_rate(params)) {
Expand Down Expand Up @@ -1339,7 +1326,33 @@ static int rt700_pcm_hw_params(struct snd_pcm_substream *substream,
snd_soc_component_write(component, RT700_ADC_FORMAT_L, val);

dev_err(dai->dev, "5 %s %s", __func__, dai->name);
return retval;
return 0;
}

static int rt700_prepare_stream(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct snd_soc_component *component = dai->component;
struct rt700_priv *rt700 = snd_soc_component_get_drvdata(component);
struct sdw_stream_data *stream;
int retval;

stream = snd_soc_dai_get_dma_data(dai, substream);

if (!stream) {
dev_err(dai->dev, "no dma data found in dai %s", dai->name);
return -ENOMEM;
}

retval = sdw_stream_add_slave(rt700->slave, &rt700->stream_config,
&rt700->port_config, 1,
stream->sdw_stream);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully get this code, is this going to initialize the 'stream'?
Again please add comments to explain what you are doing, the code is not self-explanatory.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RanderWang yeah same here. I dont understand why we need to move this to prepare. AFAICT, sdw_stream_add_slave() need the stream_config to be set, particularly the frame_rate.

if (retval) {
dev_err(dai->dev, "Unable to configure port\n");
return retval;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RanderWang just return retval outside the if. No need for 2 separate return statements here

}

return 0;
}

static int rt700_pcm_hw_free(struct snd_pcm_substream *substream,
Expand All @@ -1365,6 +1378,7 @@ static struct snd_soc_dai_ops rt700_ops = {
.hw_params = rt700_pcm_hw_params,
.hw_free = rt700_pcm_hw_free,
.set_sdw_stream = rt700_set_sdw_stream,
.prepare = rt700_prepare_stream,
.shutdown = rt700_shutdown,
};

Expand Down
2 changes: 2 additions & 0 deletions sound/soc/codecs/rt700.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ struct rt700_priv {
int dbg_payload;
enum sdw_slave_status status;
struct sdw_bus_params params;
struct sdw_stream_config stream_config;
struct sdw_port_config port_config;
bool hw_init;
};

Expand Down