[alsa-devel] [PATCHv4 2/2] ALSA: hda - Add driver for Tegra SoC HDA
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 two of the module params, power_save and probe_mask.
Signed-off-by: Dylan Reid dgreid@chromium.org --- Changes since v3:
Remove some runtime-pm only code from the regular pm path. --- Changes since v2:
Remove runtime PM. The on-chip codec doesn't support D3 stop clock so the pm_runtime code was never run.
--- Changes since v1:
Kconfig depends on OF and ARCH_TEGRA. Fix warning on 64 bit build. Integrate a bunch of good suggestions from Thierry, including: Add resets property to device tree entry. More consistent/better naming. Remove probe mask that isn't needed. Cleaner/more readable handling of dword write restriction.
--- .../bindings/sound/nvidia,tegra30-hda.txt | 27 + sound/pci/hda/Kconfig | 15 + sound/pci/hda/Makefile | 2 + sound/pci/hda/hda_tegra.c | 585 +++++++++++++++++++++ 4 files changed, 629 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt create mode 100644 sound/pci/hda/hda_tegra.c
diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt new file mode 100644 index 0000000..97f6832 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt @@ -0,0 +1,27 @@ +NVIDIA Tegra30 HDA controller + +Required properties: +- compatible : "nvidia,tegra30-hda" +- reg : Should contain the HDA registers location and length. +- interrupts : The interrupt from the hda controller. +- clocks : Must contain an entry for each required entry in clock-names. +- clock-names : Must include the following entries: hda, hdacodec_2x, hda2hdmi +- resets : Must contain an entry for each entry in reset-names. + See ../reset/reset.txt for details. +- reset-names : Must include the following entries: hda, hdacodec_2x, hda2hdmi + +Example: + +hda@70030000 { + compatible = "nvidia,tegra124-hda", "nvidia,tegra30-hda"; + reg = <0x0 0x70030000 0x10000>; + interrupts = <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA124_CLK_HDA>, + <&tegra_car TEGRA124_CLK_HDA2HDMI>, + <&tegra_car TEGRA124_CLK_HDA2CODEC_2X>; + clock-names = "hda", "hda2hdmi", "hdacodec_2x"; + resets = <&tegra_car 125>, /* hda */ + <&tegra_car 128>; /* hda2hdmi */ + <&tegra_car 111>, /* hda2codec_2x */ + reset-names = "hda", "hda2hdmi", "hdacodec_2x"; +}; diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig index ac17c3f..f17c016 100644 --- a/sound/pci/hda/Kconfig +++ b/sound/pci/hda/Kconfig @@ -20,6 +20,21 @@ config SND_HDA_INTEL To compile this driver as a module, choose M here: the module will be called snd-hda-intel.
+config SND_HDA_TEGRA + tristate "NVIDIA Tegra HD Audio" + depends on OF && ARCH_TEGRA + select SND_HDA + help + Say Y here to support the HDA controller present in NVIDIA + Tegra SoCs + + This options enables support for the HD Audio controller + present in some NVIDIA Tegra SoCs, used to communicate audio + to the HDMI output. + + To compile this driver as a module, choose M here: the module + will be called snd-hda-tegra. + if SND_HDA
config SND_HDA_DSP_LOADER diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile index d0d0c19..194f3093 100644 --- a/sound/pci/hda/Makefile +++ b/sound/pci/hda/Makefile @@ -1,5 +1,6 @@ snd-hda-intel-objs := hda_intel.o snd-hda-controller-objs := hda_controller.o +snd-hda-tegra-objs := hda_tegra.o # for haswell power well snd-hda-intel-$(CONFIG_SND_HDA_I915) += hda_i915.o
@@ -47,3 +48,4 @@ obj-$(CONFIG_SND_HDA_CODEC_HDMI) += snd-hda-codec-hdmi.o # otherwise the codec patches won't be hooked before the PCI probe # when built in kernel obj-$(CONFIG_SND_HDA_INTEL) += snd-hda-intel.o +obj-$(CONFIG_SND_HDA_TEGRA) += snd-hda-tegra.o diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c new file mode 100644 index 0000000..afdf5e1 --- /dev/null +++ b/sound/pci/hda/hda_tegra.c @@ -0,0 +1,585 @@ +/* + * + * Implementation of primary alsa driver code base for NVIDIA Tegra HDA. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + * + */ + +#include <linux/clk.h> +#include <linux/clocksource.h> +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/dma-mapping.h> +#include <linux/init.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/mutex.h> +#include <linux/of_device.h> +#include <linux/reboot.h> +#include <linux/slab.h> +#include <linux/time.h> + +#include <sound/core.h> +#include <sound/initval.h> + +#include "hda_codec.h" +#include "hda_controller.h" +#include "hda_priv.h" + +/* Defines for Nvidia Tegra HDA support */ +#define HDA_BAR0 0x8000 + +#define HDA_CFG_CMD 0x1004 +#define HDA_CFG_BAR0 0x1010 + +#define HDA_ENABLE_IO_SPACE (1 << 0) +#define HDA_ENABLE_MEM_SPACE (1 << 1) +#define HDA_ENABLE_BUS_MASTER (1 << 2) +#define HDA_ENABLE_SERR (1 << 8) +#define HDA_DISABLE_INTR (1 << 10) +#define HDA_BAR0_INIT_PROGRAM 0xFFFFFFFF +#define HDA_BAR0_FINAL_PROGRAM (1 << 14) + +/* IPFS */ +#define HDA_IPFS_CONFIG 0x180 +#define HDA_IPFS_EN_FPCI 0x1 + +#define HDA_IPFS_FPCI_BAR0 0x80 +#define HDA_FPCI_BAR0_START 0x40 + +#define HDA_IPFS_INTR_MASK 0x188 +#define HDA_IPFS_EN_INTR (1 << 16) + +struct hda_tegra { + struct azx chip; + struct device *dev; + struct clk *hda_clk; + struct clk *hda2codec_2x_clk; + struct clk *hda2hdmi_clk; + void __iomem *regs; +}; + +static int power_save = CONFIG_SND_HDA_POWER_SAVE_DEFAULT; +module_param(power_save, bint, 0644); +MODULE_PARM_DESC(power_save, + "Automatic power-saving timeout (in seconds, 0 = disable)."); + +/* + * DMA page allocation ops. + */ +static int dma_alloc_pages(struct azx *chip, int type, size_t size, + struct snd_dma_buffer *buf) +{ + return snd_dma_alloc_pages(type, chip->card->dev, size, buf); +} + +static void dma_free_pages(struct azx *chip, struct snd_dma_buffer *buf) +{ + snd_dma_free_pages(buf); +} + +static int substream_alloc_pages(struct azx *chip, + struct snd_pcm_substream *substream, + size_t size) +{ + struct azx_dev *azx_dev = get_azx_dev(substream); + + azx_dev->bufsize = 0; + azx_dev->period_bytes = 0; + azx_dev->format_val = 0; + return snd_pcm_lib_malloc_pages(substream, size); +} + +static int substream_free_pages(struct azx *chip, + struct snd_pcm_substream *substream) +{ + return snd_pcm_lib_free_pages(substream); +} + +/* + * 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 int shift = ((unsigned long)(addr) & 0x3) << 3; + void *dword_addr = (void *)((unsigned long)(addr) & ~0x3); + u32 v; + + v = readl(dword_addr); + v &= ~(0xffff << shift); + v |= value << shift; + writel(v, dword_addr); +} + +static u16 hda_tegra_readw(u16 *addr) +{ + unsigned int shift = ((unsigned long)(addr) & 0x3) << 3; + void *dword_addr = (void *)((unsigned long)(addr) & ~0x3); + u32 v; + + v = readl(dword_addr); + return (v >> shift) & 0xffff; +} + +static void hda_tegra_writeb(u8 value, u8 *addr) +{ + unsigned int shift = ((unsigned long)(addr) & 0x3) << 3; + void *dword_addr = (void *)((unsigned long)(addr) & ~0x3); + u32 v; + + v = readl(dword_addr); + v &= ~(0xff << shift); + v |= value << shift; + writel(v, dword_addr); +} + +static u8 hda_tegra_readb(u8 *addr) +{ + unsigned int shift = ((unsigned long)(addr) & 0x3) << 3; + void *dword_addr = (void *)((unsigned long)(addr) & ~0x3); + u32 v; + + v = readl(dword_addr); + return (v >> shift) & 0xff; +} + +static const struct hda_controller_ops hda_tegra_ops = { + .reg_writel = hda_tegra_writel, + .reg_readl = hda_tegra_readl, + .reg_writew = hda_tegra_writew, + .reg_readw = hda_tegra_readw, + .reg_writeb = hda_tegra_writeb, + .reg_readb = hda_tegra_readb, + .dma_alloc_pages = dma_alloc_pages, + .dma_free_pages = dma_free_pages, + .substream_alloc_pages = substream_alloc_pages, + .substream_free_pages = substream_free_pages, +}; + +static void hda_tegra_init(struct hda_tegra *hda) +{ + u32 v; + + /*Enable the PCI access */ + v = readl(hda->regs + HDA_IPFS_CONFIG); + v |= HDA_IPFS_EN_FPCI; + writel(v, hda->regs + HDA_IPFS_CONFIG); + + /* Enable MEM/IO space and bus master */ + v = readl(hda->regs + HDA_CFG_CMD); + v &= ~HDA_DISABLE_INTR; + v |= HDA_ENABLE_MEM_SPACE | HDA_ENABLE_IO_SPACE | + HDA_ENABLE_BUS_MASTER | HDA_ENABLE_SERR; + writel(v, hda->regs + HDA_CFG_CMD); + + writel(HDA_BAR0_INIT_PROGRAM, hda->regs + HDA_CFG_BAR0); + writel(HDA_BAR0_FINAL_PROGRAM, hda->regs + HDA_CFG_BAR0); + writel(HDA_FPCI_BAR0_START, hda->regs + HDA_IPFS_FPCI_BAR0); + + v = readl(hda->regs + HDA_IPFS_INTR_MASK); + v |= HDA_IPFS_EN_INTR; + writel(v, hda->regs + HDA_IPFS_INTR_MASK); +} + +static int hda_tegra_enable_clocks(struct hda_tegra *data) +{ + int rc; + + rc = clk_prepare_enable(data->hda_clk); + if (rc) + return rc; + rc = clk_prepare_enable(data->hda2codec_2x_clk); + if (rc) + goto disable_hda; + rc = clk_prepare_enable(data->hda2hdmi_clk); + if (rc) + goto disable_codec_2x; + + return 0; + +disable_codec_2x: + clk_disable_unprepare(data->hda2codec_2x_clk); +disable_hda: + clk_disable_unprepare(data->hda_clk); + return rc; +} + +static void hda_tegra_disable_clocks(struct hda_tegra *data) +{ + clk_disable_unprepare(data->hda2hdmi_clk); + clk_disable_unprepare(data->hda2codec_2x_clk); + clk_disable_unprepare(data->hda_clk); +} + +#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); + + 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; + + hda_tegra_enable_clocks(hda); + + /* Read STATESTS before controller reset */ + status = azx_readw(chip, STATESTS); + + hda_tegra_init(hda); + + azx_init_chip(chip, 1); + + snd_hda_resume(chip->bus); + snd_power_change_state(card, SNDRV_CTL_POWER_D0); + + return 0; +} +#endif /* CONFIG_PM_SLEEP */ + +static const struct dev_pm_ops hda_tegra_pm = { + SET_SYSTEM_SLEEP_PM_OPS(hda_tegra_suspend, hda_tegra_resume) +}; + +/* + * reboot notifier for hang-up problem at power-down + */ +static int hda_tegra_halt(struct notifier_block *nb, unsigned long event, + void *buf) +{ + struct azx *chip = container_of(nb, struct azx, reboot_notifier); + snd_hda_bus_reboot_notify(chip->bus); + azx_stop_chip(chip); + return NOTIFY_OK; +} + +static void hda_tegra_notifier_register(struct azx *chip) +{ + chip->reboot_notifier.notifier_call = hda_tegra_halt; + register_reboot_notifier(&chip->reboot_notifier); +} + +static void hda_tegra_notifier_unregister(struct azx *chip) +{ + if (chip->reboot_notifier.notifier_call) + unregister_reboot_notifier(&chip->reboot_notifier); +} + +/* + * destructor + */ +static int hda_tegra_dev_free(struct snd_device *device) +{ + int i; + struct azx *chip = device->device_data; + + hda_tegra_notifier_unregister(chip); + + if (chip->initialized) { + for (i = 0; i < chip->num_streams; i++) + azx_stream_stop(chip, &chip->azx_dev[i]); + azx_stop_chip(chip); + } + + azx_free_stream_pages(chip); + + return 0; +} + +static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev) +{ + struct hda_tegra *hda = container_of(chip, struct hda_tegra, chip); + struct device *dev = hda->dev; + struct resource *res; + int err; + + hda->hda_clk = devm_clk_get(dev, "hda"); + if (IS_ERR(hda->hda_clk)) + return PTR_ERR(hda->hda_clk); + hda->hda2codec_2x_clk = devm_clk_get(dev, "hda2codec_2x"); + if (IS_ERR(hda->hda2codec_2x_clk)) + return PTR_ERR(hda->hda2codec_2x_clk); + hda->hda2hdmi_clk = devm_clk_get(dev, "hda2hdmi"); + if (IS_ERR(hda->hda2hdmi_clk)) + return PTR_ERR(hda->hda2hdmi_clk); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + hda->regs = devm_ioremap_resource(dev, res); + if (IS_ERR(chip->remap_addr)) + return PTR_ERR(chip->remap_addr); + + chip->remap_addr = hda->regs + HDA_BAR0; + chip->addr = res->start + HDA_BAR0; + + err = hda_tegra_enable_clocks(hda); + if (err) + return err; + + hda_tegra_init(hda); + + return 0; +} + +/* + * The codecs were powered up in snd_hda_codec_new(). + * Now all initialization done, so turn them down if possible + */ +static void power_down_all_codecs(struct azx *chip) +{ + struct hda_codec *codec; + list_for_each_entry(codec, &chip->bus->codec_list, list) + snd_hda_power_down(codec); +} + +static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev) +{ + struct snd_card *card = chip->card; + int err; + unsigned short gcap; + int irq_id = platform_get_irq(pdev, 0); + + err = hda_tegra_init_chip(chip, pdev); + if (err) + return err; + + err = devm_request_irq(chip->card->dev, irq_id, azx_interrupt, + IRQF_SHARED, KBUILD_MODNAME, chip); + if (err) { + dev_err(chip->card->dev, + "unable to request IRQ %d, disabling device\n", + irq_id); + return err; + } + chip->irq = irq_id; + + synchronize_irq(chip->irq); + + gcap = azx_readw(chip, GCAP); + dev_dbg(card->dev, "chipset global capabilities = 0x%x\n", gcap); + + /* read number of streams from GCAP register instead of using + * hardcoded value + */ + chip->capture_streams = (gcap >> 8) & 0x0f; + chip->playback_streams = (gcap >> 12) & 0x0f; + if (!chip->playback_streams && !chip->capture_streams) { + /* gcap didn't give any info, switching to old method */ + chip->playback_streams = ICH6_NUM_PLAYBACK; + chip->capture_streams = ICH6_NUM_CAPTURE; + } + chip->capture_index_offset = 0; + chip->playback_index_offset = chip->capture_streams; + chip->num_streams = chip->playback_streams + chip->capture_streams; + chip->azx_dev = devm_kcalloc(card->dev, chip->num_streams, + sizeof(*chip->azx_dev), GFP_KERNEL); + if (!chip->azx_dev) + return -ENOMEM; + + err = azx_alloc_stream_pages(chip); + if (err < 0) + return err; + + /* initialize streams */ + azx_init_stream(chip); + + /* initialize chip */ + azx_init_chip(chip, 1); + + /* codec detection */ + if (!chip->codec_mask) { + dev_err(card->dev, "no codecs found!\n"); + return -ENODEV; + } + + strcpy(card->driver, "tegra-hda"); + strcpy(card->shortname, "tegra-hda"); + snprintf(card->longname, sizeof(card->longname), + "%s at 0x%lx irq %i", + card->shortname, chip->addr, chip->irq); + + return 0; +} + +/* + * constructor + */ +static int hda_tegra_create(struct snd_card *card, + unsigned int driver_caps, + const struct hda_controller_ops *hda_ops, + struct hda_tegra *hda) +{ + static struct snd_device_ops ops = { + .dev_free = hda_tegra_dev_free, + }; + struct azx *chip; + int err; + + chip = &hda->chip; + + spin_lock_init(&chip->reg_lock); + mutex_init(&chip->open_mutex); + chip->card = card; + chip->ops = hda_ops; + chip->irq = -1; + chip->driver_caps = driver_caps; + chip->driver_type = driver_caps & 0xff; + chip->dev_index = 0; + INIT_LIST_HEAD(&chip->pcm_list); + INIT_LIST_HEAD(&chip->list); + + chip->position_fix[0] = POS_FIX_AUTO; + chip->position_fix[1] = POS_FIX_AUTO; + chip->codec_probe_mask = -1; + + chip->single_cmd = false; + chip->snoop = true; + + err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops); + if (err < 0) { + dev_err(card->dev, "Error creating device\n"); + return err; + } + + return 0; +} + +static const struct of_device_id hda_tegra_match[] = { + { .compatible = "nvidia,tegra30-hda" }, + {}, +}; + +static int hda_tegra_probe(struct platform_device *pdev) +{ + struct snd_card *card; + struct azx *chip; + struct hda_tegra *hda; + const struct of_device_id *of_id; + int err; + const unsigned int driver_flags = AZX_DCAPS_RIRB_DELAY; + + of_id = of_match_device(hda_tegra_match, &pdev->dev); + if (!of_id) + return -ENODEV; + + hda = devm_kzalloc(&pdev->dev, sizeof(*hda), GFP_KERNEL); + if (!hda) + return -ENOMEM; + hda->dev = &pdev->dev; + chip = &hda->chip; + + err = snd_card_new(&pdev->dev, SNDRV_DEFAULT_IDX1, SNDRV_DEFAULT_STR1, + THIS_MODULE, 0, &card); + if (err < 0) { + dev_err(&pdev->dev, "Error creating card!\n"); + return err; + } + + err = hda_tegra_create(card, driver_flags, &hda_tegra_ops, hda); + if (err < 0) + goto out_free; + card->private_data = chip; + + dev_set_drvdata(&pdev->dev, card); + + err = hda_tegra_first_init(chip, pdev); + if (err < 0) + goto out_free; + + /* create codec instances */ + err = azx_codec_create(chip, NULL, 0, &power_save); + if (err < 0) + goto out_free; + + err = azx_codec_configure(chip); + if (err < 0) + goto out_free; + + /* create PCM streams */ + err = snd_hda_build_pcms(chip->bus); + if (err < 0) + goto out_free; + + /* create mixer controls */ + err = azx_mixer_create(chip); + if (err < 0) + goto out_free; + + err = snd_card_register(chip->card); + if (err < 0) + goto out_free; + + chip->running = 1; + power_down_all_codecs(chip); + hda_tegra_notifier_register(chip); + + return 0; + +out_free: + snd_card_free(card); + return err; +} + +static int hda_tegra_remove(struct platform_device *pdev) +{ + return snd_card_free(dev_get_drvdata(&pdev->dev)); +} + +static struct platform_driver tegra_platform_hda = { + .driver = { + .name = "tegra-hda", + .pm = &hda_tegra_pm, + .of_match_table = hda_tegra_match, + }, + .probe = hda_tegra_probe, + .remove = hda_tegra_remove, +}; +module_platform_driver(tegra_platform_hda); + +MODULE_DESCRIPTION("Tegra HDA bus driver"); +MODULE_LICENSE("GPL v2"); +MODULE_DEVICE_TABLE(of, tegra_platform_hda_match);
On 04/18/2014 03:46 PM, 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 two of the module params, power_save and probe_mask.
(I'm curious how this was tested using an upstream kernel considering we don't have HDMI support on Tegra124 enabled yet. If you added 1 or 2 more CODEC IDs to patch 1/2, you could probably test on an earlier SoC generation)
Sorry for the slow review...
diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt
+- compatible : "nvidia,tegra30-hda" +- reg : Should contain the HDA registers location and length. +- interrupts : The interrupt from the hda controller.
hda should be capitalized.
+- clocks : Must contain an entry for each required entry in clock-names.
Can you add the following line after that for consistency with other Tegra bindings:
See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entries: hda, hdacodec_2x, hda2hdmi +- resets : Must contain an entry for each entry in reset-names.
- See ../reset/reset.txt for details.
+- reset-names : Must include the following entries: hda, hdacodec_2x, hda2hdmi
+Example:
+hda@70030000 {
that should be named hda@0,70030000, since the reg property's value below assumes the parent has #address-cells=<2>.
- compatible = "nvidia,tegra124-hda", "nvidia,tegra30-hda";
- reg = <0x0 0x70030000 0x10000>;
... and here, since #address-cells=<2>, then #size-cells should be 2 too, so that should be:
reg = <0x0 0x70030000 0 0x10000>;
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
+config SND_HDA_TEGRA
- tristate "NVIDIA Tegra HD Audio"
- depends on OF && ARCH_TEGRA
OF is selected by ARCH_TEGRA, so this only needs to depend on ARCH_TEGRA.
(Of course, you could make this depend on ARCH_TEGRA || (COMPILE_TEST && OF && ...)
diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c +/*
- Implementation of primary alsa driver code base for NVIDIA Tegra HDA.
ALSA should be capitalized.
+static void hda_tegra_init(struct hda_tegra *hda) +{
- u32 v;
- /*Enable the PCI access */
There should be a space after /*.
+static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev)
- err = hda_tegra_enable_clocks(hda);
- if (err)
return err;
I'm not sure where the matching disable() occurs? Is the card assumed to be started in a powered state, so the next PM transition would be hda_tegra_suspend()? IIRC, other Tegra devices with PM start in a power-saved state, and hence would leave clocks stopped after probe(). It's fine if that's the reason; it just looks different so I'm making sure that's what is going on.
+static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
- /* read number of streams from GCAP register instead of using
* hardcoded value
*/
- chip->capture_streams = (gcap >> 8) & 0x0f;
- chip->playback_streams = (gcap >> 12) & 0x0f;
- if (!chip->playback_streams && !chip->capture_streams) {
/* gcap didn't give any info, switching to old method */
chip->playback_streams = ICH6_NUM_PLAYBACK;
chip->capture_streams = ICH6_NUM_CAPTURE;
Are ICH6_* defines appropriate for Tegra?
+static int hda_tegra_probe(struct platform_device *pdev)
- of_id = of_match_device(hda_tegra_match, &pdev->dev);
- if (!of_id)
return -ENODEV;
Since of_id isn't used anywhere, there's no point calling of_match_device() to look it up. The driver core won't call hda_tegra_probe() unless there is a matching entry in the table.
+MODULE_DEVICE_TABLE(of, tegra_platform_hda_match);
I think it's typical to put that line immediately after the table it applies to. Not a big deal though.
With those minor issues fixed, Reviewed-by: Stephen Warren swarren@nvidia.com
On Mon, Apr 21, 2014 at 1:02 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/18/2014 03:46 PM, 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 two of the module params, power_save and probe_mask.
(I'm curious how this was tested using an upstream kernel considering we don't have HDMI support on Tegra124 enabled yet. If you added 1 or 2 more CODEC IDs to patch 1/2, you could probably test on an earlier SoC generation)
Luckily this applies to our downstream tree with hdmi enabled. The only change needed is switching back from snd_card_new to snd_card_create. So that's where I've been doing my testing.
I think we've got a tegra30 eval board. I'll dig it up tomorrow and give it a try on linux-next.
Sorry for the slow review...
diff --git a/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt
+- compatible : "nvidia,tegra30-hda" +- reg : Should contain the HDA registers location and length. +- interrupts : The interrupt from the hda controller.
hda should be capitalized.
+- clocks : Must contain an entry for each required entry in clock-names.
Can you add the following line after that for consistency with other Tegra bindings:
See ../clocks/clock-bindings.txt for details.
+- clock-names : Must include the following entries: hda, hdacodec_2x, hda2hdmi +- resets : Must contain an entry for each entry in reset-names.
- See ../reset/reset.txt for details.
+- reset-names : Must include the following entries: hda, hdacodec_2x, hda2hdmi
+Example:
+hda@70030000 {
that should be named hda@0,70030000, since the reg property's value below assumes the parent has #address-cells=<2>.
compatible = "nvidia,tegra124-hda", "nvidia,tegra30-hda";
reg = <0x0 0x70030000 0x10000>;
... and here, since #address-cells=<2>, then #size-cells should be 2 too, so that should be:
reg = <0x0 0x70030000 0 0x10000>;
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
+config SND_HDA_TEGRA
tristate "NVIDIA Tegra HD Audio"
depends on OF && ARCH_TEGRA
OF is selected by ARCH_TEGRA, so this only needs to depend on ARCH_TEGRA.
(Of course, you could make this depend on ARCH_TEGRA || (COMPILE_TEST && OF && ...)
diff --git a/sound/pci/hda/hda_tegra.c b/sound/pci/hda/hda_tegra.c +/*
- Implementation of primary alsa driver code base for NVIDIA Tegra HDA.
ALSA should be capitalized.
+static void hda_tegra_init(struct hda_tegra *hda) +{
u32 v;
/*Enable the PCI access */
There should be a space after /*.
+static int hda_tegra_init_chip(struct azx *chip, struct platform_device *pdev)
err = hda_tegra_enable_clocks(hda);
if (err)
return err;
I'm not sure where the matching disable() occurs? Is the card assumed to be started in a powered state, so the next PM transition would be hda_tegra_suspend()? IIRC, other Tegra devices with PM start in a power-saved state, and hence would leave clocks stopped after probe(). It's fine if that's the reason; it just looks different so I'm making sure that's what is going on.
Yes, this starts in a powered state. The first chance to power down is after the call to "power_down_all_codecs" that might arm a power_save timeout and then power down the controller. However the HDMI codec here doesn't report that it supports D3 stop clock so the controller will stay powered until it is suspended.
+static int hda_tegra_first_init(struct azx *chip, struct platform_device *pdev)
/* read number of streams from GCAP register instead of using
* hardcoded value
*/
chip->capture_streams = (gcap >> 8) & 0x0f;
chip->playback_streams = (gcap >> 12) & 0x0f;
if (!chip->playback_streams && !chip->capture_streams) {
/* gcap didn't give any info, switching to old method */
chip->playback_streams = ICH6_NUM_PLAYBACK;
chip->capture_streams = ICH6_NUM_CAPTURE;
Are ICH6_* defines appropriate for Tegra?
+static int hda_tegra_probe(struct platform_device *pdev)
of_id = of_match_device(hda_tegra_match, &pdev->dev);
if (!of_id)
return -ENODEV;
Since of_id isn't used anywhere, there's no point calling of_match_device() to look it up. The driver core won't call hda_tegra_probe() unless there is a matching entry in the table.
+MODULE_DEVICE_TABLE(of, tegra_platform_hda_match);
I think it's typical to put that line immediately after the table it applies to. Not a big deal though.
With those minor issues fixed, Reviewed-by: Stephen Warren swarren@nvidia.com
Thanks for reviewing this Stephen, I'll run a test on the Tegra30 tomorrow and post a new version.
-Dylan
participants (2)
-
Dylan Reid
-
Stephen Warren