On 2/26/18 3:25 AM, Takashi Iwai wrote:
On Mon, 26 Feb 2018 11:11:44 +0100,
Ughreja, Rakesh A wrote:
-----Original Message-----
From: Takashi Iwai [mailto:tiwai@xxxxxxx]
Sent: Friday, February 23, 2018 3:48 PM
To: Ughreja, Rakesh A <rakesh.a.ughreja@xxxxxxxxx>
Cc: alsa-devel@xxxxxxxxxxxxxxxx; broonie@xxxxxxxxxx;
liam.r.girdwood@xxxxxxxxxxxxxxx; pierre-louis.bossart@xxxxxxxxxxxxxxx; Koul,
Vinod <vinod.koul@xxxxxxxxx>; Patches Audio <patches.audio@xxxxxxxxx>
Subject: Re: [PATCH v1 8/9] ASoC: hdac_hda: add asoc extension for legacy HDA
codec drivers
On Fri, 23 Feb 2018 09:12:29 +0100,
Rakesh Ughreja wrote:
+static int hdac_hda_codec_probe(struct snd_soc_codec *codec)
+{
+ struct hdac_hda_priv *hda_pvt = snd_soc_codec_get_drvdata(codec);
+ struct snd_soc_dapm_context *dapm =
+ snd_soc_component_get_dapm(&codec-
component);
+ struct hdac_device *hdev = &hda_pvt->codec.core;
+ struct hda_codec *hcodec = &hda_pvt->codec;
+ struct hdac_ext_link *hlink = NULL;
+ hda_codec_patch_t patch;
+ int ret, i = 0;
+ u16 codec_mask;
+
+ hda_pvt->scodec = codec;
+ hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev));
+ if (!hlink) {
+ dev_err(&hdev->dev, "hdac link not found\n");
+ return -EIO;
+ }
+
+ ret = snd_hdac_ext_bus_link_get(hdev->bus, hlink);
So this is the essential part for the ext-hda stuff. But...
+
+ udelay(1000);
+ do {
+ codec_mask = snd_hdac_chip_readw(hdev->bus, STATESTS);
+ if (codec_mask)
+ break;
+ i++;
+ udelay(100);
+ } while (i < 100);
+
+ if (!codec_mask) {
+ dev_err(&hdev->dev, "No codecs found after link reset\n");
+ return -EIO;
+ }
+
+ snd_hdac_chip_writew(hdev->bus, STATESTS, STATESTS_INT_MASK);
Why do you need this? The callback gets called by the HD-audio
controller driver and it already should have checked the codec mask
bits.
With the multiple link support on the same HDA controller, when the link
gets turned ON immediately codec may not indicate its presence. As per
the HDA spec, it may take up to 521uSec before the codec responds.
In the legacy HDA the link was turned ON/OFF only during CRST time, but
now it can happen anytime.
This must be documented. Otherwise no one understands it.
And I still don't think such stuff belonging to the codec driver.
It's rather a job of the controller driver to assure the codec access.
If any, we should fix that side instead.
I missed what the 'multiple-link' notion means or which part of the spec
this is referring to?
The code here assumes one analog codec and one iDisp, there isn't any
support for additional codecs.
What am I missing?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
