Re: [PATCH v13 3/3] ASoC: tda998x: add a codec to the HDMI transmitter
On Tue, 28 Jul 2015 15:53:58 +0200, Russell King - ARM Linux wrote: > > On Tue, Jul 28, 2015 at 03:23:29PM +0200, Jean-Francois Moine wrote: > > The EDID arrives in the DRM connector when video starts. The built ELD > > may be stored either in the connector itself (default), in the encoder > > (TDA998x device) or in some DRM/encoder/connector private data. > > The ELD is stored in the DRM connector, and there _should_ be a system > of locking which ensures that you can get at that information safely. > > The ELD is only updated when the connector's get_modes() method is called, > and the driver itself calls drm_edid_to_eld(). This all happens under > the drm_device's struct_mutex. > > So, to safely read the ELD from outside DRM, you need to take the top-level > drm_device's struct_mutex. That could be fraught, as that's quite a big > lock, so an alternative would be to introduce an 'eld' lock to the driver, > which protects the call to drm_edid_to_eld() and the reading of the ELD. > > However, that doesn't solve the problem of passing the data to an audio > driver. What's needed there is a notification system which allows a video > driver to broadcast HDMI events such as: > > * connected > * new EDID available > * new ELD available > * disconnected > > With such a system, different components driving the facilities of a HDMI > connector can make decisions on what to do - which not only includes things > like the audio driver, but also a driver for the CEC interface (which also > needs to see the EDID to get at its "address".) This can be done in a safe > manner as the HDMI video driver would have control over the generation of > these messages. > > The point that Mark is trying to get you to see is that this problem is > not specific to TDA998x. The same problem exists for many other HDMI > interfaces (except maybe Intel's i9x5/HDA stuff which has a hardware > mechanism of passing the ELD data from the video driver, through the > hardware to the audio driver.) FWIW, we're currently discussing about extending the i915/hda component binding so that the audio driver gets a notification and queries the ELD via callbacks instead of the flaky hardware access. It'd be best if we have a common infrastructure for that, of course. But currently it's a start, just a bit step forward there. Takashi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/7] ALSA: pcm: Add snd_interval_ranges() and snd_pcm_hw_constraint_ranges()
At Wed, 28 Jan 2015 15:16:06 +0100,
Peter Rosin wrote:
>
> From: Peter Rosin
>
> Add helper functions to allow drivers to specify several disjoint
> ranges for a variable. In particular, there is a codec (PCM512x) that
> has a hole in its supported range of rates, due to PLL and divider
> restrictions.
>
> This is like snd_pcm_hw_constraint_list(), but for ranges instead of
> points.
>
> Signed-off-by: Peter Rosin
> Reviewed-by: Lars-Peter Clausen
Mark, feel free to take my ack if you carry this with other patches
through your tree.
Reviewed-by: Takashi Iwai
thanks,
Takashi
> ---
> include/sound/pcm.h | 12 +++
> sound/core/pcm_lib.c | 85
> ++
> 2 files changed, 97 insertions(+)
>
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 1e7f74acc2ec..04fc037e0555 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -275,6 +275,12 @@ struct snd_pcm_hw_constraint_list {
> unsigned int mask;
> };
>
> +struct snd_pcm_hw_constraint_ranges {
> + unsigned int count;
> + const struct snd_interval *ranges;
> + unsigned int mask;
> +};
> +
> struct snd_pcm_hwptr_log;
>
> struct snd_pcm_runtime {
> @@ -910,6 +916,8 @@ void snd_interval_mulkdiv(const struct snd_interval *a,
> unsigned int k,
> const struct snd_interval *b, struct snd_interval *c);
> int snd_interval_list(struct snd_interval *i, unsigned int count,
> const unsigned int *list, unsigned int mask);
> +int snd_interval_ranges(struct snd_interval *i, unsigned int count,
> + const struct snd_interval *list, unsigned int mask);
> int snd_interval_ratnum(struct snd_interval *i,
> unsigned int rats_count, struct snd_ratnum *rats,
> unsigned int *nump, unsigned int *denp);
> @@ -934,6 +942,10 @@ int snd_pcm_hw_constraint_list(struct snd_pcm_runtime
> *runtime,
> unsigned int cond,
> snd_pcm_hw_param_t var,
> const struct snd_pcm_hw_constraint_list *l);
> +int snd_pcm_hw_constraint_ranges(struct snd_pcm_runtime *runtime,
> + unsigned int cond,
> + snd_pcm_hw_param_t var,
> + const struct snd_pcm_hw_constraint_ranges *r);
> int snd_pcm_hw_constraint_ratnums(struct snd_pcm_runtime *runtime,
> unsigned int cond,
> snd_pcm_hw_param_t var,
> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
> index ec9e7866177f..446c00bd908b 100644
> --- a/sound/core/pcm_lib.c
> +++ b/sound/core/pcm_lib.c
> @@ -1015,6 +1015,60 @@ int snd_interval_list(struct snd_interval *i, unsigned
> int count,
>
> EXPORT_SYMBOL(snd_interval_list);
>
> +/**
> + * snd_interval_ranges - refine the interval value from the list of ranges
> + * @i: the interval value to refine
> + * @count: the number of elements in the list of ranges
> + * @ranges: the ranges list
> + * @mask: the bit-mask to evaluate
> + *
> + * Refines the interval value from the list of ranges.
> + * When mask is non-zero, only the elements corresponding to bit 1 are
> + * evaluated.
> + *
> + * Return: Positive if the value is changed, zero if it's not changed, or a
> + * negative error code.
> + */
> +int snd_interval_ranges(struct snd_interval *i, unsigned int count,
> + const struct snd_interval *ranges, unsigned int mask)
> +{
> + unsigned int k;
> + struct snd_interval range_union;
> + struct snd_interval range;
> +
> + if (!count) {
> + snd_interval_none(i);
> + return -EINVAL;
> + }
> + snd_interval_any(&range_union);
> + range_union.min = UINT_MAX;
> + range_union.max = 0;
> + for (k = 0; k < count; k++) {
> + if (mask && !(mask & (1 << k)))
> + continue;
> + snd_interval_copy(&range, &ranges[k]);
> + if (snd_interval_refine(&range, i) < 0)
> + continue;
> + if (snd_interval_empty(&range))
> + continue;
> +
> + if (range.min < range_union.min) {
> + range_union.min = range.min;
> + range_union.openmin = 1;
> + }
> + if (range.min == range_union.min && !range.openmin)
> + range_union.openmin = 0;
> + if (range.max > range_union.max) {
> +
Re: [alsa-devel] [PATCH] ASoC: add xtensa xtfpga I2S interface and platform
At Tue, 28 Oct 2014 17:39:13 +0100,
Lars-Peter Clausen wrote:
>
> On 10/28/2014 05:04 PM, Mark Brown wrote:
> > On Tue, Oct 28, 2014 at 04:58:24PM +0100, Lars-Peter Clausen wrote:
> >> On 10/27/2014 08:07 PM, Max Filippov wrote:
> >
> >>> + if (tx_active) {
> >>> + if (i2s->tx_fifo_high < 256)
> >>> + xtfpga_i2s_refill_fifo(i2s);
> >>> + else
> >>> + tasklet_hi_schedule(&i2s->refill_fifo);
> >
> >> Maybe use threaded IRQs instead of IRQ + tasklet.
> >
> > Is that going to play nicely with the fact that the interrupt can be
> > shared and the desire to (AFAICT) do NAPI style stuff with the interrupt
> > disabled for long periods?
> >
>
> Threaded interrupts got support for interrupt sharing a while ago, so I
> guess yes. I think it will even work better than the tasklet approach. You
> can configure the IRQ to disable itself as long as the thread is running.
Yes, I tested the threaded irq with PCI drivers using shared irqs and
worked well.
Takashi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
At Tue, 02 Sep 2014 16:12:40 +0530,
Varka Bhadram wrote:
>
> On 09/02/2014 04:08 PM, Jean-Francois Moine wrote:
> > On Tue, 02 Sep 2014 15:51:41 +0530
> > Varka Bhadram wrote:
> >
> >>> + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> >>> + case 0x11:
> >>> + return SND_SOC_DAIFMT_CBS_CFS;
> >>> + case 0x10:
> >>> + return SND_SOC_DAIFMT_CBS_CFM;
> >>> + case 0x01:
> >>> + return SND_SOC_DAIFMT_CBM_CFS;
> >>> + default:
> >>> + return SND_SOC_DAIFMT_CBM_CFM;
> >>> + }
> >>> +
> >>> + /* Shouldn't be here */
> >>> + return -EINVAL;
> >>> +}
> >> It will be nice if we declare the switch case numbers as macros (specific
> >> name)...
> > I don't see which macros: the values are just 2 booleans.
> >
> I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
> names to those...?
The whole switch block is too hackish, makes unnecessarily complex.
It can be more strightforwardly like:
if (np == bitclkmaster)
return np == framemater ?
SND_SOC_DAIFMT_CBS_CFS : SND_SOC_DAIFMT_CBS_CFM;
else
return np == framemaster ?
SND_SOC_DAIFMT_CBM_CFS : SND_SOC_DAIFMT_CBM_CFM;
Or, if you love efficiency and complexity, something like:
#define SND_SOC_DAIFMT(_np, _clk, _frame) \
_np) != (_clk)) | (((_np) != (_frame)) << 1) << 12) + (1 << 12)
Then
return SND_SOC_DAIFMT(np, blkclkmaster, framemaster);
Takashi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv5 2/2] ALSA: hda - Add driver for Tegra SoC HDA
At Mon, 19 May 2014 19:18:27 -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 two of the module params, power_save and probe_mask.
>
> Signed-off-by: Dylan Reid
> Reviewed-by: Stephen Warren
> ---
> Changes since v4:
> Fix up binding doc, Kconfig depends and some style issue suggested by
> swarren. Test on Venice2 which now supports HDMI output.
>
> 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.
I applied the patch now to for-next branch. Let's continue
development on top of that. Thanks.
Takashi
> ---
> .../bindings/sound/nvidia,tegra30-hda.txt | 28 +
> sound/pci/hda/Kconfig | 15 +
> sound/pci/hda/Makefile | 2 +
> sound/pci/hda/hda_tegra.c | 584
> +
> 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 000..c26f8ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/nvidia,tegra30-hda.txt
> @@ -0,0 +1,28 @@
> +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.
> + 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@0,7003 {
> + compatible = "nvidia,tegra124-hda", "nvidia,tegra30-hda";
> + reg = <0x0 0x7003 0x0 0x1>;
> + interrupts = ;
> + 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..ebf4c2f 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 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 000..e472bc8
> --- /dev/null
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -0,0 +1,584 @@
> +/*
> + *
> + * Implementation of primary ALSA driver code base for NVIDIA Tegra HDA.
Re: [PATCHv3 2/2] ALSA: hda - Add driver for Tegra SoC HDA
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
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 &= ~(0x << 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Device tree support for Atmel AC97 controller on AT91SAM9263
At Tue, 15 Apr 2014 00:47:11 +0200, Alexandre Belloni wrote: > > On 14/04/2014 at 20:43:02 +0200, Alexander Stein wrote : > > Hello Takashi, > > > > On Monday 14 April 2014, 10:42:06 wrote Takashi Iwai: > > > At Sat, 12 Apr 2014 11:08:23 +0200, > > > Alexander Stein wrote: > > > > > > > > This set of patches add device tree support for the AC97 controller > > > > found > > > > on AT91SAM9263. > > > > The first two patches are minor cleanup, while the last ones add the > > actual > > > > support. > > > > > > Do you really need work on this driver? The sound/atmel/* stuff is an > > > old one before ASoC was introduced, mostly obsoleted nowadays. > > > > Ah, ok. I wasn't aware of that. But is there a replacement for the ac97c > > driver? All I found in sound/soc/atmel is using the SSC peripheral. Not > > something I can use. > > I guess what Takashi wanted to say was: shouldn't we rewrite the driver > completely to take advantage of the ASoC infrastructure ? Yep, exactly. Thanks for my sparse word completion :) > I'm actually quite interesting in getting it to work as this is the la > peripheral that doesn't have DT support on the SAM9RL. > > I'll make some comments on your patches but I'm not sure whether we > should keep that driver or move to ASoC. Maybe that could be an > intermediate step. I'll try to have a look this week. OK, if people think it's still useful, I can take the patches without problems. The changes are small and easy enough. But, the only condition is that it's assured that the changes / binding is applicable to ASoC (in future, if any). That said, it'd be great if anyone can work on the proper integration to ASoC at the same time. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Device tree support for Atmel AC97 controller on AT91SAM9263
At Sat, 12 Apr 2014 11:08:23 +0200, Alexander Stein wrote: > > This set of patches add device tree support for the AC97 controller found > on AT91SAM9263. > The first two patches are minor cleanup, while the last ones add the actual > support. Do you really need work on this driver? The sound/atmel/* stuff is an old one before ASoC was introduced, mostly obsoleted nowadays. Takashi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ALSA: hda - Add driver for Tegra SoC HDA
At Wed, 9 Apr 2014 11:22:13 +0200,
Thierry Reding wrote:
>
> > +
> > +static int probe_mask[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1};
> > +module_param_array(probe_mask, int, NULL, 0444);
> > +MODULE_PARM_DESC(probe_mask, "Bitmask to probe codecs (default = -1).");
>
> Is this really necessary? Do we need this with Tegra? Can't we simply
> assume that there's always only one codec? Or always use a probe_mask of
> -1? Alternatively, wouldn't this be better off in DT?
This is an option rather for debugging. Usually the codec is
recognized automatically in the case of HD-audio.
But, another point is, suppose this is really specific to Tegra and
nothing else, whether we need multiple instances. If there is only a
single controller, we can omit the arrays.
Takashi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ALSA: hda - Add driver for Tegra SoC HDA
At Tue, 8 Apr 2014 12:15:33 -0700,
Dylan Reid wrote:
>
> diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
> index ac17c3f..4f6ee6b 100644
> --- a/sound/pci/hda/Kconfig
> +++ b/sound/pci/hda/Kconfig
> @@ -20,6 +20,20 @@ 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 "Tegra HD Audio"
> + select SND_HDA
Does (or should) this be build generically? Or any arch-specific
dependency, or maybe at least "depends on OF"?
> + 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 000..c36bbca
> --- /dev/null
> +++ b/sound/pci/hda/hda_tegra.c
> @@ -0,0 +1,695 @@
> +/*
> + *
> + * Implementation of primary alsa driver code base for Tegra HDA.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that 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.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include "hda_codec.h"
> +#include "hda_controller.h"
> +#include "hda_priv.h"
> +
> +#define DRV_NAME "tegra-hda"
> +
> +/* Defines for Nvidia Tegra HDA support */
> +#define NVIDIA_TEGRA_HDA_BAR0_OFFSET 0x8000
> +
> +#define NVIDIA_TEGRA_HDA_CFG_CMD_OFFSET0x1004
> +#define NVIDIA_TEGRA_HDA_CFG_BAR0_OFFSET 0x1010
> +
> +#define NVIDIA_TEGRA_HDA_ENABLE_IO_SPACE (1 << 0)
> +#define NVIDIA_TEGRA_HDA_ENABLE_MEM_SPACE (1 << 1)
> +#define NVIDIA_TEGRA_HDA_ENABLE_BUS_MASTER (1 << 2)
> +#define NVIDIA_TEGRA_HDA_ENABLE_SERR (1 << 8)
> +#define NVIDIA_TEGRA_HDA_DISABLE_INTR (1 << 10)
> +#define NVIDIA_TEGRA_HDA_BAR0_INIT_PROGRAM 0x
> +#define NVIDIA_TEGRA_HDA_BAR0_FINAL_PROGRAM(1 << 14)
> +
> +/* IPFS */
> +#define NVIDIA_TEGRA_HDA_IPFS_CONFIG 0x180
> +#define NVIDIA_TEGRA_HDA_IPFS_EN_FPCI 0x1
> +
> +#define NVIDIA_TEGRA_HDA_IPFS_FPCI_BAR00x80
> +#define NVIDIA_TEGRA_HDA_FPCI_BAR0_START 0x40
> +
> +#define NVIDIA_TEGRA_HDA_IPFS_INTR_MASK0x188
> +#define NVIDIA_TEGRA_HDA_IPFS_EN_INTR (1 << 16)
> +
> +struct hda_tegra_data {
> + struct azx chip;
> + struct platform_device *pdev;
> + struct clk **platform_clks;
> + int platform_clk_count;
> + void __iomem *remap_config_addr;
> +};
> +
> +static int probe_mask[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS-1)] = -1};
> +module_param_array(probe_mask, int, NULL, 0444);
> +MODULE_PARM_DESC(probe_mask, "Bitmask to probe codecs (default = -1).");
> +
> +static int power_save = CONFIG_SND_HDA_POWER_SAVE_DEFAULT;
> +static int *power_save_addr = &power_save;
> +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, s
Re: [alsa-devel] [PATCH] ASoC: pcm512x: Add PCM512x driver
At Thu, 06 Feb 2014 14:16:23 +0100, Lars-Peter Clausen wrote: > > On 02/06/2014 02:12 PM, Takashi Iwai wrote: > > At Thu, 6 Feb 2014 13:07:35 +, > > Mark Brown wrote: > [...] > >> Hrm, it is but this points out an error in the control helpers which has > >> been there since forever - they call that parameter max but it's not a > >> maximum, it's the number of elements in the enumeration. I bet we have > >> a bunch of other enumerations which miss the last element as a result. > > > > Yeah, the argument name is really confusing. I had to double-check > > the code when I reviewed your patch, too :) > > > > Also it'd be better to have a practice to use either ARRAY_SIZE() or a > > constant there, too. > > There is also SOC_ENUM_SINGLE_DECL(...) which takes care of the common case > where you want as many items as your array contains. Ah, right, that makes life easier. Takashi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] ASoC: pcm512x: Add PCM512x driver
At Thu, 6 Feb 2014 13:07:35 +,
Mark Brown wrote:
>
> On Thu, Feb 06, 2014 at 01:51:33PM +0100, Takashi Iwai wrote:
> > Mark Brown wrote:
>
> > > +static const char *pcm512x_dsp_program_texts[] = {
>
> > The numbers of items in pcm512x_dsp_program_texts[] and _values[]
> > don't match.
>
> Yeah, fixed.
I'm thinking whether we can check this in the macro.
I thought of using BUILD_BUG_ON(), but it's unsure whether it aligns
there well.
> > > +static const struct soc_enum pcm512x_clk_missing =
> > > + SOC_ENUM_SINGLE(PCM512x_CLKDET, 0, 7, pcm512x_clk_missing_text);
>
> > Isn't it 8?
>
> Hrm, it is but this points out an error in the control helpers which has
> been there since forever - they call that parameter max but it's not a
> maximum, it's the number of elements in the enumeration. I bet we have
> a bunch of other enumerations which miss the last element as a result.
Yeah, the argument name is really confusing. I had to double-check
the code when I reviewed your patch, too :)
Also it'd be better to have a practice to use either ARRAY_SIZE() or a
constant there, too.
Takashi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] ASoC: pcm512x: Add PCM512x driver
At Thu, 6 Feb 2014 12:26:15 +,
Mark Brown wrote:
> diff --git a/sound/soc/codecs/pcm512x.c b/sound/soc/codecs/pcm512x.c
(snip)
> +static const char *pcm512x_dsp_program_texts[] = {
> + "FIR interpolation with de-emphasis",
> + "Low latency IIR with de-emphasis",
> + "High attenuation with de-emphasis",
> + "Ringing-less low latency FIR",
> +};
> +
> +static const unsigned int pcm512x_dsp_program_values[] = {
> + 1,
> + 2,
> + 3,
> + 5,
> + 7,
> +};
The numbers of items in pcm512x_dsp_program_texts[] and _values[]
don't match.
> +static const SOC_VALUE_ENUM_SINGLE_DECL(pcm512x_dsp_program,
> + PCM512x_DSP_PROGRAM, 0, 0x1f,
> + pcm512x_dsp_program_texts,
> + pcm512x_dsp_program_values);
> +
> +static const char *pcm512x_clk_missing_text[] = {
> + "1s", "2s", "3s", "4s", "5s", "6s", "7s", "8s"
> +};
> +
> +static const struct soc_enum pcm512x_clk_missing =
> + SOC_ENUM_SINGLE(PCM512x_CLKDET, 0, 7, pcm512x_clk_missing_text);
Isn't it 8?
> +
> +static const char *pcm512x_autom_text[] = {
> + "21ms", "106ms", "213ms", "533ms", "1.07s", "2.13s", "5.33s", "10.66s"
> +};
> +
> +static const struct soc_enum pcm512x_autom_l =
> + SOC_ENUM_SINGLE(PCM512x_AUTO_MUTE, PCM512x_ATML_SHIFT, 7,
> + pcm512x_autom_text);
> +
> +static const struct soc_enum pcm512x_autom_r =
> + SOC_ENUM_SINGLE(PCM512x_AUTO_MUTE, PCM512x_ATMR_SHIFT, 7,
> + pcm512x_autom_text);
Ditto.
Takashi
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
