Skip to content

ASoC: sof: bxt_pcm512x adds device_link#783

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

Closed
libinyang wants to merge 1 commit into from
Closed

ASoC: sof: bxt_pcm512x adds device_link #783

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 26 additions & 0 deletions sound/soc/intel/boards/bxt_pcm512x.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,6 +26,7 @@

struct bxt_card_private {
struct list_head hdmi_pcm_list;
struct device_link *link;
};

#if IS_ENABLED(CONFIG_SND_SOC_HDAC_HDMI)
Expand DownExpand Up@@ -53,6 +54,19 @@ static int broxton_hdmi_init(struct snd_soc_pcm_runtime *rtd)

list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);

/*
* Setup the device_link to make sure that hdmi codec power is
* turned on before the consumer starts to operate on the
* codec register.
*/
if (!ctx->link)
ctx->link = device_link_add(rtd->card->dev, dai->dev,

@ranj063ranj063Apr 4, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@libinyang looks like we need to check for null after device_link_add as it could return null?

@libinyanglibinyangApr 4, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If it returns NULL, it means the link is not added. But I don't think we should stop the init. Or we add a dev_warn?

@ranj063ranj063Apr 4, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm a warning will be good I think

DL_FLAG_RPM_ACTIVE);

@lyakhlyakhApr 8, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wondering, wouldn't adding a DL_FLAG_AUTOREMOVE_CONSUMER flag would eliminate the need for bxt_pcm512x_remove() below? Or maybe even also add DL_FLAG_AUTOREMOVE_SUPPLIER?

@libinyanglibinyangApr 8, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@lyakh I checked the DL_FLAG_AUTOREMOVE_xxx before. I found very little drivers are using these flags. What's more, looking into the code, what I understand is that we still need call some functions to release the link even we use the flags? Based on the result, I didn't add these flags for safe. And it seems Takashi is OK for using the flag DL_FLAG_RPM_ACTIVE.


if (!ctx->link)
dev_warn(rtd->card->dev,
"failed creating device_link for HDMI codec\n");

return 0;
}

Expand DownExpand Up@@ -303,12 +317,24 @@ static int bxt_pcm512x_probe(struct platform_device *pdev)
return ret_val;
}

int bxt_pcm512x_remove(struct platform_device *pdev)
{
struct snd_soc_card *card = platform_get_drvdata(pdev);
struct bxt_card_private *ctx = snd_soc_card_get_drvdata(card);

if (ctx->link)
device_link_del(ctx->link);

return 0;
}

static struct platform_driver bxt_pcm521x_driver = {
.driver = {
.name = "bxt-pcm512x",
.pm = &snd_soc_pm_ops,
},
.probe = bxt_pcm512x_probe,
.remove = bxt_pcm512x_remove,
};
module_platform_driver(bxt_pcm521x_driver);

Expand Down