At Wed, 16 Apr 2014 14:39:32 -0700, Dylan Reid wrote:
This adds a driver for the HDA block in Tegra SoCs. The HDA bus is used to communicate with the HDMI codec on Tegra124.
Most of the code is re-used from the Intel/PCI HDA driver. It brings over only the power_save module parameter.
Signed-off-by: Dylan Reid dgreid@chromium.org
Thanks, this looks better, but a few comments below. Besides, I'd like to have a review in DT part.
+/*
- Register access ops. Tegra HDA register access is DWORD only.
- */
+static void hda_tegra_writel(u32 value, u32 *addr) +{
- writel(value, addr);
+}
+static u32 hda_tegra_readl(u32 *addr) +{
- return readl(addr);
+}
+static void hda_tegra_writew(u16 value, u16 *addr) +{
- unsigned long shift = ((uintptr_t)(addr) & 0x3) << 3;
- void *dword_addr = (void *)((uintptr_t)(addr) & ~0x3);
- u32 v;
- v = readl(dword_addr);
- v &= ~(0xffff << shift);
- v |= value << shift;
- writel(v, dword_addr);
+}
The shift doesn't have to be long at all, and the cast in kernel is usually just unsigned long instead of uintptr_t.
+#ifdef CONFIG_PM_SLEEP +/*
- power management
- */
+static int hda_tegra_suspend(struct device *dev) +{
- struct snd_card *card = dev_get_drvdata(dev);
- struct azx *chip = card->private_data;
- struct azx_pcm *p;
- struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
- snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
- list_for_each_entry(p, &chip->pcm_list, list)
snd_pcm_suspend_all(p->pcm);- if (chip->initialized)
snd_hda_suspend(chip->bus);- /* enable controller wake up event */
- azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) |
STATESTS_INT_MASK);
I guess WAKEEN doesn't work for Tegra?
- azx_stop_chip(chip);
- azx_enter_link_reset(chip);
- hda_tegra_disable_clocks(hda);
- return 0;
+}
+static int hda_tegra_resume(struct device *dev) +{
- struct snd_card *card = dev_get_drvdata(dev);
- struct azx *chip = card->private_data;
- struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip);
- int status;
- struct hda_codec *codec;
- hda_tegra_enable_clocks(hda);
- /* Read STATESTS before controller reset */
- status = azx_readw(chip, STATESTS);
- hda_tegra_init(hda);
- if (status && chip->bus) {
list_for_each_entry(codec, &chip->bus->codec_list, list)if (status & (1 << codec->addr))queue_delayed_work(codec->bus->workq,&codec->jackpoll_work,codec->jackpoll_interval);- }
This is superfluous for the regular resume. It was some missing call only in the runtime resume.
- /* disable controller Wake Up event*/
- azx_writew(chip, WAKEEN, azx_readw(chip, WAKEEN) & ~STATESTS_INT_MASK);
This can be removed like the suspend.
thanks,
Takashi