[alsa-devel] [PATCH V2 0/3] ASOC: bcm2835: move bcm2835-i2s to use clock framework
From: Martin Sperl kernel@martin.sperl.org
This patchset enables the bcm2835-i2s driver to use the clock framework which was introduced with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.").
This commit resulted in the fact that the bcm2835-i2s driver was no longer working due to some register addresses used by 2 drivers (clk-bcm2835 and bcm2835-i2s).
This patchset requires that the patchset for PCM-clock support as well as fractional/mash support is applied to the clk-bcm2835 driver and the corresponding device tree, but as the current version of the driver is not working, that should not be a problem.
Note that there is one change: right now the current driver tries to calculate an optimal bclk_ratio based on its knowledge that it is using the 19.2Mhz oscillator. This computation would recommend the use of 40 or 80 bits instead of 32/64 bits that are required.
Some of the DACs can not handle this, so most downstream drivers would set snd_soc_dai_set_bclk_ratio explicitly to disable this "non-power-of-2" automatic selection.
So it seems wise to leave it out of the current patchset.
If it is deemed necessary, then I can provide a separate patch that implements this again.
Changelog: V1->V2: * moving clock patches into a separate patchset, which fixes also other issues in the clock framework * remove unnecessary bclk_size assignments
Martin Sperl (3): ASoC: bcm2835: move to use the clock framework ARM: bcm2835: I2S: use new register-range and clock framework dt-bindings: bsm2835: fix bindings documentation to use new clock framework
.../devicetree/bindings/sound/brcm,bcm2835-i2s.txt | 7 +- arch/arm/boot/dts/bcm2835.dtsi | 5 +- sound/soc/bcm/bcm2835-i2s.c | 284 +++++--------------- 3 files changed, 69 insertions(+), 227 deletions(-)
-- 1.7.10.4
From: Martin Sperl kernel@martin.sperl.org
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") this driver was no longer functional as it was manipulating the clock registers locally without going true the framework.
This patch moves to use the new clock framework and also moves away from the hardcoded address offsets for DMA getting the dma-address directly from the device tree.
Note that the optimal bclk_ratio selection to avoid jitter due to the use of fractional dividers, which is in the current version has been removed, because not all devices support these non power of 2 sized transfers, which resulted in lots of (downstream) modules that use: snd_soc_dai_set_bclk_ratio(cpu_dai, sample_bits * 2);
Signed-off-by: Martin Sperl kernel@martin.sperl.org --- sound/soc/bcm/bcm2835-i2s.c | 284 ++++++++++--------------------------------- 1 file changed, 64 insertions(+), 220 deletions(-)
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c index 3303d5f..1c1f221 100644 --- a/sound/soc/bcm/bcm2835-i2s.c +++ b/sound/soc/bcm/bcm2835-i2s.c @@ -37,6 +37,7 @@ #include <linux/init.h> #include <linux/io.h> #include <linux/module.h> +#include <linux/of_address.h> #include <linux/slab.h>
#include <sound/core.h> @@ -46,55 +47,6 @@ #include <sound/pcm_params.h> #include <sound/soc.h>
-/* Clock registers */ -#define BCM2835_CLK_PCMCTL_REG 0x00 -#define BCM2835_CLK_PCMDIV_REG 0x04 - -/* Clock register settings */ -#define BCM2835_CLK_PASSWD (0x5a000000) -#define BCM2835_CLK_PASSWD_MASK (0xff000000) -#define BCM2835_CLK_MASH(v) ((v) << 9) -#define BCM2835_CLK_FLIP BIT(8) -#define BCM2835_CLK_BUSY BIT(7) -#define BCM2835_CLK_KILL BIT(5) -#define BCM2835_CLK_ENAB BIT(4) -#define BCM2835_CLK_SRC(v) (v) - -#define BCM2835_CLK_SHIFT (12) -#define BCM2835_CLK_DIVI(v) ((v) << BCM2835_CLK_SHIFT) -#define BCM2835_CLK_DIVF(v) (v) -#define BCM2835_CLK_DIVF_MASK (0xFFF) - -enum { - BCM2835_CLK_MASH_0 = 0, - BCM2835_CLK_MASH_1, - BCM2835_CLK_MASH_2, - BCM2835_CLK_MASH_3, -}; - -enum { - BCM2835_CLK_SRC_GND = 0, - BCM2835_CLK_SRC_OSC, - BCM2835_CLK_SRC_DBG0, - BCM2835_CLK_SRC_DBG1, - BCM2835_CLK_SRC_PLLA, - BCM2835_CLK_SRC_PLLC, - BCM2835_CLK_SRC_PLLD, - BCM2835_CLK_SRC_HDMI, -}; - -/* Most clocks are not useable (freq = 0) */ -static const unsigned int bcm2835_clk_freq[BCM2835_CLK_SRC_HDMI+1] = { - [BCM2835_CLK_SRC_GND] = 0, - [BCM2835_CLK_SRC_OSC] = 19200000, - [BCM2835_CLK_SRC_DBG0] = 0, - [BCM2835_CLK_SRC_DBG1] = 0, - [BCM2835_CLK_SRC_PLLA] = 0, - [BCM2835_CLK_SRC_PLLC] = 0, - [BCM2835_CLK_SRC_PLLD] = 500000000, - [BCM2835_CLK_SRC_HDMI] = 0, -}; - /* I2S registers */ #define BCM2835_I2S_CS_A_REG 0x00 #define BCM2835_I2S_FIFO_A_REG 0x04 @@ -158,10 +110,6 @@ static const unsigned int bcm2835_clk_freq[BCM2835_CLK_SRC_HDMI+1] = { #define BCM2835_I2S_INT_RXR BIT(1) #define BCM2835_I2S_INT_TXW BIT(0)
-/* I2S DMA interface */ -/* FIXME: Needs IOMMU support */ -#define BCM2835_VCMMU_SHIFT (0x7E000000 - 0x20000000) - /* General device struct */ struct bcm2835_i2s_dev { struct device *dev; @@ -169,21 +117,23 @@ struct bcm2835_i2s_dev { unsigned int fmt; unsigned int bclk_ratio;
- struct regmap *i2s_regmap; - struct regmap *clk_regmap; + struct regmap *i2s_regmap; + struct clk *clk; + bool clk_prepared; };
static void bcm2835_i2s_start_clock(struct bcm2835_i2s_dev *dev) { - /* Start the clock if in master mode */ unsigned int master = dev->fmt & SND_SOC_DAIFMT_MASTER_MASK;
+ if (dev->clk_prepared) + return; + switch (master) { case SND_SOC_DAIFMT_CBS_CFS: case SND_SOC_DAIFMT_CBS_CFM: - regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, - BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB, - BCM2835_CLK_PASSWD | BCM2835_CLK_ENAB); + clk_prepare_enable(dev->clk); + dev->clk_prepared = true; break; default: break; @@ -192,28 +142,9 @@ static void bcm2835_i2s_start_clock(struct bcm2835_i2s_dev *dev)
static void bcm2835_i2s_stop_clock(struct bcm2835_i2s_dev *dev) { - uint32_t clkreg; - int timeout = 1000; - - /* Stop clock */ - regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, - BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB, - BCM2835_CLK_PASSWD); - - /* Wait for the BUSY flag going down */ - while (--timeout) { - regmap_read(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, &clkreg); - if (!(clkreg & BCM2835_CLK_BUSY)) - break; - } - - if (!timeout) { - /* KILL the clock */ - dev_err(dev->dev, "I2S clock didn't stop. Kill the clock!\n"); - regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, - BCM2835_CLK_KILL | BCM2835_CLK_PASSWD_MASK, - BCM2835_CLK_KILL | BCM2835_CLK_PASSWD); - } + if (dev->clk_prepared) + clk_disable_unprepare(dev->clk); + dev->clk_prepared = false; }
static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, @@ -223,8 +154,7 @@ static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, uint32_t syncval; uint32_t csreg; uint32_t i2s_active_state; - uint32_t clkreg; - uint32_t clk_active_state; + bool clk_was_prepared; uint32_t off; uint32_t clr;
@@ -238,15 +168,10 @@ static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg); i2s_active_state = csreg & (BCM2835_I2S_RXON | BCM2835_I2S_TXON);
- regmap_read(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, &clkreg); - clk_active_state = clkreg & BCM2835_CLK_ENAB; - /* Start clock if not running */ - if (!clk_active_state) { - regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, - BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB, - BCM2835_CLK_PASSWD | BCM2835_CLK_ENAB); - } + clk_was_prepared = dev->clk_prepared; + if (!clk_was_prepared) + bcm2835_i2s_start_clock(dev);
/* Stop I2S module */ regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, off, 0); @@ -280,7 +205,7 @@ static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, dev_err(dev->dev, "I2S SYNC error!\n");
/* Stop clock if it was not running before */ - if (!clk_active_state) + if (!clk_was_prepared) bcm2835_i2s_stop_clock(dev);
/* Restore I2S state */ @@ -309,19 +234,9 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai); - unsigned int sampling_rate = params_rate(params); unsigned int data_length, data_delay, bclk_ratio; unsigned int ch1pos, ch2pos, mode, format; - unsigned int mash = BCM2835_CLK_MASH_1; - unsigned int divi, divf, target_frequency; - int clk_src = -1; - unsigned int master = dev->fmt & SND_SOC_DAIFMT_MASTER_MASK; - bool bit_master = (master == SND_SOC_DAIFMT_CBS_CFS - || master == SND_SOC_DAIFMT_CBS_CFM); - - bool frame_master = (master == SND_SOC_DAIFMT_CBS_CFS - || master == SND_SOC_DAIFMT_CBM_CFS); uint32_t csreg;
/* @@ -343,11 +258,9 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream, switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE: data_length = 16; - bclk_ratio = 40; break; case SNDRV_PCM_FORMAT_S32_LE: data_length = 32; - bclk_ratio = 80; break; default: return -EINVAL; @@ -356,69 +269,12 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream, /* If bclk_ratio already set, use that one. */ if (dev->bclk_ratio) bclk_ratio = dev->bclk_ratio; + else + /* otherwise calculate a fitting block ratio */ + bclk_ratio = 2 * data_length;
- /* - * Clock Settings - * - * The target frequency of the bit clock is - * sampling rate * frame length - * - * Integer mode: - * Sampling rates that are multiples of 8000 kHz - * can be driven by the oscillator of 19.2 MHz - * with an integer divider as long as the frame length - * is an integer divider of 19200000/8000=2400 as set up above. - * This is no longer possible if the sampling rate - * is too high (e.g. 192 kHz), because the oscillator is too slow. - * - * MASH mode: - * For all other sampling rates, it is not possible to - * have an integer divider. Approximate the clock - * with the MASH module that induces a slight frequency - * variance. To minimize that it is best to have the fastest - * clock here. That is PLLD with 500 MHz. - */ - target_frequency = sampling_rate * bclk_ratio; - clk_src = BCM2835_CLK_SRC_OSC; - mash = BCM2835_CLK_MASH_0; - - if (bcm2835_clk_freq[clk_src] % target_frequency == 0 - && bit_master && frame_master) { - divi = bcm2835_clk_freq[clk_src] / target_frequency; - divf = 0; - } else { - uint64_t dividend; - - if (!dev->bclk_ratio) { - /* - * Overwrite bclk_ratio, because the - * above trick is not needed or can - * not be used. - */ - bclk_ratio = 2 * data_length; - } - - target_frequency = sampling_rate * bclk_ratio; - - clk_src = BCM2835_CLK_SRC_PLLD; - mash = BCM2835_CLK_MASH_1; - - dividend = bcm2835_clk_freq[clk_src]; - dividend <<= BCM2835_CLK_SHIFT; - do_div(dividend, target_frequency); - divi = dividend >> BCM2835_CLK_SHIFT; - divf = dividend & BCM2835_CLK_DIVF_MASK; - } - - /* Set clock divider */ - regmap_write(dev->clk_regmap, BCM2835_CLK_PCMDIV_REG, BCM2835_CLK_PASSWD - | BCM2835_CLK_DIVI(divi) - | BCM2835_CLK_DIVF(divf)); - - /* Setup clock, but don't start it yet */ - regmap_write(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, BCM2835_CLK_PASSWD - | BCM2835_CLK_MASH(mash) - | BCM2835_CLK_SRC(clk_src)); + /* set target clock rate*/ + clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
/* Setup the frame format */ format = BCM2835_I2S_CHEN; @@ -692,7 +548,7 @@ static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = { .trigger = bcm2835_i2s_trigger, .hw_params = bcm2835_i2s_hw_params, .set_fmt = bcm2835_i2s_set_dai_fmt, - .set_bclk_ratio = bcm2835_i2s_set_dai_bclk_ratio + .set_bclk_ratio = bcm2835_i2s_set_dai_bclk_ratio, };
static int bcm2835_i2s_dai_probe(struct snd_soc_dai *dai) @@ -750,34 +606,14 @@ static bool bcm2835_i2s_precious_reg(struct device *dev, unsigned int reg) }; }
-static bool bcm2835_clk_volatile_reg(struct device *dev, unsigned int reg) -{ - switch (reg) { - case BCM2835_CLK_PCMCTL_REG: - return true; - default: - return false; - }; -} - -static const struct regmap_config bcm2835_regmap_config[] = { - { - .reg_bits = 32, - .reg_stride = 4, - .val_bits = 32, - .max_register = BCM2835_I2S_GRAY_REG, - .precious_reg = bcm2835_i2s_precious_reg, - .volatile_reg = bcm2835_i2s_volatile_reg, - .cache_type = REGCACHE_RBTREE, - }, - { - .reg_bits = 32, - .reg_stride = 4, - .val_bits = 32, - .max_register = BCM2835_CLK_PCMDIV_REG, - .volatile_reg = bcm2835_clk_volatile_reg, - .cache_type = REGCACHE_RBTREE, - }, +static const struct regmap_config bcm2835_regmap_config = { + .reg_bits = 32, + .reg_stride = 4, + .val_bits = 32, + .max_register = BCM2835_I2S_GRAY_REG, + .precious_reg = bcm2835_i2s_precious_reg, + .volatile_reg = bcm2835_i2s_volatile_reg, + .cache_type = REGCACHE_RBTREE, };
static const struct snd_soc_component_driver bcm2835_i2s_component = { @@ -787,42 +623,50 @@ static const struct snd_soc_component_driver bcm2835_i2s_component = { static int bcm2835_i2s_probe(struct platform_device *pdev) { struct bcm2835_i2s_dev *dev; - int i; int ret; - struct regmap *regmap[2]; - struct resource *mem[2]; - - /* Request both ioareas */ - for (i = 0; i <= 1; i++) { - void __iomem *base; - - mem[i] = platform_get_resource(pdev, IORESOURCE_MEM, i); - base = devm_ioremap_resource(&pdev->dev, mem[i]); - if (IS_ERR(base)) - return PTR_ERR(base); - - regmap[i] = devm_regmap_init_mmio(&pdev->dev, base, - &bcm2835_regmap_config[i]); - if (IS_ERR(regmap[i])) - return PTR_ERR(regmap[i]); - } + struct resource *mem; + void __iomem *base; + const __be32 *addr; + dma_addr_t dma_base;
dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM;
- dev->i2s_regmap = regmap[0]; - dev->clk_regmap = regmap[1]; + /* get the clock */ + dev->clk_prepared = false; + dev->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(dev->clk)) { + dev_err(&pdev->dev, "could not get clk: %ld\n", + PTR_ERR(dev->clk)); + return PTR_ERR(dev->clk); + } + + /* Request ioarea */ + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + base = devm_ioremap_resource(&pdev->dev, mem); + if (IS_ERR(base)) + return PTR_ERR(base); + + dev->i2s_regmap = devm_regmap_init_mmio(&pdev->dev, base, + &bcm2835_regmap_config); + if (IS_ERR(dev->i2s_regmap)) + return PTR_ERR(dev->i2s_regmap); + + /* Set the DMA address - we have to parse DT ourselves */ + addr = of_get_address(pdev->dev.of_node, 0, NULL, NULL); + if (!addr) { + dev_err(&pdev->dev, "could not get DMA-register address\n"); + return -EINVAL; + } + dma_base = be32_to_cpup(addr);
- /* Set the DMA address */ dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr = - (dma_addr_t)mem[0]->start + BCM2835_I2S_FIFO_A_REG - + BCM2835_VCMMU_SHIFT; + dma_base + BCM2835_I2S_FIFO_A_REG;
dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].addr = - (dma_addr_t)mem[0]->start + BCM2835_I2S_FIFO_A_REG - + BCM2835_VCMMU_SHIFT; + dma_base + BCM2835_I2S_FIFO_A_REG;
/* Set the bus width */ dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr_width =
The patch
ASoC: bcm2835: move to use the clock framework
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 517e7a1537ae4663268be5d0c0ec62c563b9fc99 Mon Sep 17 00:00:00 2001
From: Martin Sperl kernel@martin.sperl.org Date: Tue, 12 Jan 2016 12:35:46 +0000 Subject: [PATCH] ASoC: bcm2835: move to use the clock framework
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") this driver was no longer functional as it was manipulating the clock registers locally without going true the framework.
This patch moves to use the new clock framework and also moves away from the hardcoded address offsets for DMA getting the dma-address directly from the device tree.
Note that the optimal bclk_ratio selection to avoid jitter due to the use of fractional dividers, which is in the current version has been removed, because not all devices support these non power of 2 sized transfers, which resulted in lots of (downstream) modules that use: snd_soc_dai_set_bclk_ratio(cpu_dai, sample_bits * 2);
Signed-off-by: Martin Sperl kernel@martin.sperl.org Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/bcm/bcm2835-i2s.c | 284 ++++++++++---------------------------------- 1 file changed, 64 insertions(+), 220 deletions(-)
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c index 3303d5f58082..1c1f2210387b 100644 --- a/sound/soc/bcm/bcm2835-i2s.c +++ b/sound/soc/bcm/bcm2835-i2s.c @@ -37,6 +37,7 @@ #include <linux/init.h> #include <linux/io.h> #include <linux/module.h> +#include <linux/of_address.h> #include <linux/slab.h>
#include <sound/core.h> @@ -46,55 +47,6 @@ #include <sound/pcm_params.h> #include <sound/soc.h>
-/* Clock registers */ -#define BCM2835_CLK_PCMCTL_REG 0x00 -#define BCM2835_CLK_PCMDIV_REG 0x04 - -/* Clock register settings */ -#define BCM2835_CLK_PASSWD (0x5a000000) -#define BCM2835_CLK_PASSWD_MASK (0xff000000) -#define BCM2835_CLK_MASH(v) ((v) << 9) -#define BCM2835_CLK_FLIP BIT(8) -#define BCM2835_CLK_BUSY BIT(7) -#define BCM2835_CLK_KILL BIT(5) -#define BCM2835_CLK_ENAB BIT(4) -#define BCM2835_CLK_SRC(v) (v) - -#define BCM2835_CLK_SHIFT (12) -#define BCM2835_CLK_DIVI(v) ((v) << BCM2835_CLK_SHIFT) -#define BCM2835_CLK_DIVF(v) (v) -#define BCM2835_CLK_DIVF_MASK (0xFFF) - -enum { - BCM2835_CLK_MASH_0 = 0, - BCM2835_CLK_MASH_1, - BCM2835_CLK_MASH_2, - BCM2835_CLK_MASH_3, -}; - -enum { - BCM2835_CLK_SRC_GND = 0, - BCM2835_CLK_SRC_OSC, - BCM2835_CLK_SRC_DBG0, - BCM2835_CLK_SRC_DBG1, - BCM2835_CLK_SRC_PLLA, - BCM2835_CLK_SRC_PLLC, - BCM2835_CLK_SRC_PLLD, - BCM2835_CLK_SRC_HDMI, -}; - -/* Most clocks are not useable (freq = 0) */ -static const unsigned int bcm2835_clk_freq[BCM2835_CLK_SRC_HDMI+1] = { - [BCM2835_CLK_SRC_GND] = 0, - [BCM2835_CLK_SRC_OSC] = 19200000, - [BCM2835_CLK_SRC_DBG0] = 0, - [BCM2835_CLK_SRC_DBG1] = 0, - [BCM2835_CLK_SRC_PLLA] = 0, - [BCM2835_CLK_SRC_PLLC] = 0, - [BCM2835_CLK_SRC_PLLD] = 500000000, - [BCM2835_CLK_SRC_HDMI] = 0, -}; - /* I2S registers */ #define BCM2835_I2S_CS_A_REG 0x00 #define BCM2835_I2S_FIFO_A_REG 0x04 @@ -158,10 +110,6 @@ static const unsigned int bcm2835_clk_freq[BCM2835_CLK_SRC_HDMI+1] = { #define BCM2835_I2S_INT_RXR BIT(1) #define BCM2835_I2S_INT_TXW BIT(0)
-/* I2S DMA interface */ -/* FIXME: Needs IOMMU support */ -#define BCM2835_VCMMU_SHIFT (0x7E000000 - 0x20000000) - /* General device struct */ struct bcm2835_i2s_dev { struct device *dev; @@ -169,21 +117,23 @@ struct bcm2835_i2s_dev { unsigned int fmt; unsigned int bclk_ratio;
- struct regmap *i2s_regmap; - struct regmap *clk_regmap; + struct regmap *i2s_regmap; + struct clk *clk; + bool clk_prepared; };
static void bcm2835_i2s_start_clock(struct bcm2835_i2s_dev *dev) { - /* Start the clock if in master mode */ unsigned int master = dev->fmt & SND_SOC_DAIFMT_MASTER_MASK;
+ if (dev->clk_prepared) + return; + switch (master) { case SND_SOC_DAIFMT_CBS_CFS: case SND_SOC_DAIFMT_CBS_CFM: - regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, - BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB, - BCM2835_CLK_PASSWD | BCM2835_CLK_ENAB); + clk_prepare_enable(dev->clk); + dev->clk_prepared = true; break; default: break; @@ -192,28 +142,9 @@ static void bcm2835_i2s_start_clock(struct bcm2835_i2s_dev *dev)
static void bcm2835_i2s_stop_clock(struct bcm2835_i2s_dev *dev) { - uint32_t clkreg; - int timeout = 1000; - - /* Stop clock */ - regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, - BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB, - BCM2835_CLK_PASSWD); - - /* Wait for the BUSY flag going down */ - while (--timeout) { - regmap_read(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, &clkreg); - if (!(clkreg & BCM2835_CLK_BUSY)) - break; - } - - if (!timeout) { - /* KILL the clock */ - dev_err(dev->dev, "I2S clock didn't stop. Kill the clock!\n"); - regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, - BCM2835_CLK_KILL | BCM2835_CLK_PASSWD_MASK, - BCM2835_CLK_KILL | BCM2835_CLK_PASSWD); - } + if (dev->clk_prepared) + clk_disable_unprepare(dev->clk); + dev->clk_prepared = false; }
static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, @@ -223,8 +154,7 @@ static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, uint32_t syncval; uint32_t csreg; uint32_t i2s_active_state; - uint32_t clkreg; - uint32_t clk_active_state; + bool clk_was_prepared; uint32_t off; uint32_t clr;
@@ -238,15 +168,10 @@ static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg); i2s_active_state = csreg & (BCM2835_I2S_RXON | BCM2835_I2S_TXON);
- regmap_read(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, &clkreg); - clk_active_state = clkreg & BCM2835_CLK_ENAB; - /* Start clock if not running */ - if (!clk_active_state) { - regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, - BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB, - BCM2835_CLK_PASSWD | BCM2835_CLK_ENAB); - } + clk_was_prepared = dev->clk_prepared; + if (!clk_was_prepared) + bcm2835_i2s_start_clock(dev);
/* Stop I2S module */ regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, off, 0); @@ -280,7 +205,7 @@ static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, dev_err(dev->dev, "I2S SYNC error!\n");
/* Stop clock if it was not running before */ - if (!clk_active_state) + if (!clk_was_prepared) bcm2835_i2s_stop_clock(dev);
/* Restore I2S state */ @@ -309,19 +234,9 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai); - unsigned int sampling_rate = params_rate(params); unsigned int data_length, data_delay, bclk_ratio; unsigned int ch1pos, ch2pos, mode, format; - unsigned int mash = BCM2835_CLK_MASH_1; - unsigned int divi, divf, target_frequency; - int clk_src = -1; - unsigned int master = dev->fmt & SND_SOC_DAIFMT_MASTER_MASK; - bool bit_master = (master == SND_SOC_DAIFMT_CBS_CFS - || master == SND_SOC_DAIFMT_CBS_CFM); - - bool frame_master = (master == SND_SOC_DAIFMT_CBS_CFS - || master == SND_SOC_DAIFMT_CBM_CFS); uint32_t csreg;
/* @@ -343,11 +258,9 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream, switch (params_format(params)) { case SNDRV_PCM_FORMAT_S16_LE: data_length = 16; - bclk_ratio = 40; break; case SNDRV_PCM_FORMAT_S32_LE: data_length = 32; - bclk_ratio = 80; break; default: return -EINVAL; @@ -356,69 +269,12 @@ static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream, /* If bclk_ratio already set, use that one. */ if (dev->bclk_ratio) bclk_ratio = dev->bclk_ratio; + else + /* otherwise calculate a fitting block ratio */ + bclk_ratio = 2 * data_length;
- /* - * Clock Settings - * - * The target frequency of the bit clock is - * sampling rate * frame length - * - * Integer mode: - * Sampling rates that are multiples of 8000 kHz - * can be driven by the oscillator of 19.2 MHz - * with an integer divider as long as the frame length - * is an integer divider of 19200000/8000=2400 as set up above. - * This is no longer possible if the sampling rate - * is too high (e.g. 192 kHz), because the oscillator is too slow. - * - * MASH mode: - * For all other sampling rates, it is not possible to - * have an integer divider. Approximate the clock - * with the MASH module that induces a slight frequency - * variance. To minimize that it is best to have the fastest - * clock here. That is PLLD with 500 MHz. - */ - target_frequency = sampling_rate * bclk_ratio; - clk_src = BCM2835_CLK_SRC_OSC; - mash = BCM2835_CLK_MASH_0; - - if (bcm2835_clk_freq[clk_src] % target_frequency == 0 - && bit_master && frame_master) { - divi = bcm2835_clk_freq[clk_src] / target_frequency; - divf = 0; - } else { - uint64_t dividend; - - if (!dev->bclk_ratio) { - /* - * Overwrite bclk_ratio, because the - * above trick is not needed or can - * not be used. - */ - bclk_ratio = 2 * data_length; - } - - target_frequency = sampling_rate * bclk_ratio; - - clk_src = BCM2835_CLK_SRC_PLLD; - mash = BCM2835_CLK_MASH_1; - - dividend = bcm2835_clk_freq[clk_src]; - dividend <<= BCM2835_CLK_SHIFT; - do_div(dividend, target_frequency); - divi = dividend >> BCM2835_CLK_SHIFT; - divf = dividend & BCM2835_CLK_DIVF_MASK; - } - - /* Set clock divider */ - regmap_write(dev->clk_regmap, BCM2835_CLK_PCMDIV_REG, BCM2835_CLK_PASSWD - | BCM2835_CLK_DIVI(divi) - | BCM2835_CLK_DIVF(divf)); - - /* Setup clock, but don't start it yet */ - regmap_write(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, BCM2835_CLK_PASSWD - | BCM2835_CLK_MASH(mash) - | BCM2835_CLK_SRC(clk_src)); + /* set target clock rate*/ + clk_set_rate(dev->clk, sampling_rate * bclk_ratio);
/* Setup the frame format */ format = BCM2835_I2S_CHEN; @@ -692,7 +548,7 @@ static const struct snd_soc_dai_ops bcm2835_i2s_dai_ops = { .trigger = bcm2835_i2s_trigger, .hw_params = bcm2835_i2s_hw_params, .set_fmt = bcm2835_i2s_set_dai_fmt, - .set_bclk_ratio = bcm2835_i2s_set_dai_bclk_ratio + .set_bclk_ratio = bcm2835_i2s_set_dai_bclk_ratio, };
static int bcm2835_i2s_dai_probe(struct snd_soc_dai *dai) @@ -750,34 +606,14 @@ static bool bcm2835_i2s_precious_reg(struct device *dev, unsigned int reg) }; }
-static bool bcm2835_clk_volatile_reg(struct device *dev, unsigned int reg) -{ - switch (reg) { - case BCM2835_CLK_PCMCTL_REG: - return true; - default: - return false; - }; -} - -static const struct regmap_config bcm2835_regmap_config[] = { - { - .reg_bits = 32, - .reg_stride = 4, - .val_bits = 32, - .max_register = BCM2835_I2S_GRAY_REG, - .precious_reg = bcm2835_i2s_precious_reg, - .volatile_reg = bcm2835_i2s_volatile_reg, - .cache_type = REGCACHE_RBTREE, - }, - { - .reg_bits = 32, - .reg_stride = 4, - .val_bits = 32, - .max_register = BCM2835_CLK_PCMDIV_REG, - .volatile_reg = bcm2835_clk_volatile_reg, - .cache_type = REGCACHE_RBTREE, - }, +static const struct regmap_config bcm2835_regmap_config = { + .reg_bits = 32, + .reg_stride = 4, + .val_bits = 32, + .max_register = BCM2835_I2S_GRAY_REG, + .precious_reg = bcm2835_i2s_precious_reg, + .volatile_reg = bcm2835_i2s_volatile_reg, + .cache_type = REGCACHE_RBTREE, };
static const struct snd_soc_component_driver bcm2835_i2s_component = { @@ -787,42 +623,50 @@ static const struct snd_soc_component_driver bcm2835_i2s_component = { static int bcm2835_i2s_probe(struct platform_device *pdev) { struct bcm2835_i2s_dev *dev; - int i; int ret; - struct regmap *regmap[2]; - struct resource *mem[2]; - - /* Request both ioareas */ - for (i = 0; i <= 1; i++) { - void __iomem *base; - - mem[i] = platform_get_resource(pdev, IORESOURCE_MEM, i); - base = devm_ioremap_resource(&pdev->dev, mem[i]); - if (IS_ERR(base)) - return PTR_ERR(base); - - regmap[i] = devm_regmap_init_mmio(&pdev->dev, base, - &bcm2835_regmap_config[i]); - if (IS_ERR(regmap[i])) - return PTR_ERR(regmap[i]); - } + struct resource *mem; + void __iomem *base; + const __be32 *addr; + dma_addr_t dma_base;
dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL); if (!dev) return -ENOMEM;
- dev->i2s_regmap = regmap[0]; - dev->clk_regmap = regmap[1]; + /* get the clock */ + dev->clk_prepared = false; + dev->clk = devm_clk_get(&pdev->dev, NULL); + if (IS_ERR(dev->clk)) { + dev_err(&pdev->dev, "could not get clk: %ld\n", + PTR_ERR(dev->clk)); + return PTR_ERR(dev->clk); + } + + /* Request ioarea */ + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + base = devm_ioremap_resource(&pdev->dev, mem); + if (IS_ERR(base)) + return PTR_ERR(base); + + dev->i2s_regmap = devm_regmap_init_mmio(&pdev->dev, base, + &bcm2835_regmap_config); + if (IS_ERR(dev->i2s_regmap)) + return PTR_ERR(dev->i2s_regmap); + + /* Set the DMA address - we have to parse DT ourselves */ + addr = of_get_address(pdev->dev.of_node, 0, NULL, NULL); + if (!addr) { + dev_err(&pdev->dev, "could not get DMA-register address\n"); + return -EINVAL; + } + dma_base = be32_to_cpup(addr);
- /* Set the DMA address */ dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr = - (dma_addr_t)mem[0]->start + BCM2835_I2S_FIFO_A_REG - + BCM2835_VCMMU_SHIFT; + dma_base + BCM2835_I2S_FIFO_A_REG;
dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].addr = - (dma_addr_t)mem[0]->start + BCM2835_I2S_FIFO_A_REG - + BCM2835_VCMMU_SHIFT; + dma_base + BCM2835_I2S_FIFO_A_REG;
/* Set the bus width */ dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr_width =
kernel@martin.sperl.org writes:
From: Martin Sperl kernel@martin.sperl.org
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") this driver was no longer functional as it was manipulating the clock registers locally without going true the framework.
This patch moves to use the new clock framework and also moves away from the hardcoded address offsets for DMA getting the dma-address directly from the device tree.
Note that the optimal bclk_ratio selection to avoid jitter due to the use of fractional dividers, which is in the current version has been removed, because not all devices support these non power of 2 sized transfers, which resulted in lots of (downstream) modules that use: snd_soc_dai_set_bclk_ratio(cpu_dai, sample_bits * 2);
Signed-off-by: Martin Sperl kernel@martin.sperl.org
sound/soc/bcm/bcm2835-i2s.c | 284 ++++++++++--------------------------------- 1 file changed, 64 insertions(+), 220 deletions(-)
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c index 3303d5f..1c1f221 100644 --- a/sound/soc/bcm/bcm2835-i2s.c +++ b/sound/soc/bcm/bcm2835-i2s.c
- dev->i2s_regmap = regmap[0];
- dev->clk_regmap = regmap[1];
- /* get the clock */
- dev->clk_prepared = false;
- dev->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(dev->clk)) {
dev_err(&pdev->dev, "could not get clk: %ld\n",PTR_ERR(dev->clk));return PTR_ERR(dev->clk);- }
- /* Request ioarea */
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(&pdev->dev, mem);
- if (IS_ERR(base))
return PTR_ERR(base);- dev->i2s_regmap = devm_regmap_init_mmio(&pdev->dev, base,
&bcm2835_regmap_config);- if (IS_ERR(dev->i2s_regmap))
return PTR_ERR(dev->i2s_regmap);- /* Set the DMA address - we have to parse DT ourselves */
- addr = of_get_address(pdev->dev.of_node, 0, NULL, NULL);
- if (!addr) {
dev_err(&pdev->dev, "could not get DMA-register address\n");return -EINVAL;- }
- dma_base = be32_to_cpup(addr);
Why aren't we just using mem->start like before? That seems like an independent change that should be justified on its own. I'd be ready to ack the patch if that change is removed.
I'm also trying to work my way through the clock changes.
On 28.01.2016 23:08, Eric Anholt wrote:
kernel@martin.sperl.org writes:
From: Martin Sperl kernel@martin.sperl.org
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") this driver was no longer functional as it was manipulating the clock registers locally without going true the framework.
This patch moves to use the new clock framework and also moves away from the hardcoded address offsets for DMA getting the dma-address directly from the device tree.
Note that the optimal bclk_ratio selection to avoid jitter due to the use of fractional dividers, which is in the current version has been removed, because not all devices support these non power of 2 sized transfers, which resulted in lots of (downstream) modules that use: snd_soc_dai_set_bclk_ratio(cpu_dai, sample_bits * 2);
Signed-off-by: Martin Sperl kernel@martin.sperl.org
sound/soc/bcm/bcm2835-i2s.c | 284 ++++++++++--------------------------------- 1 file changed, 64 insertions(+), 220 deletions(-)
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c index 3303d5f..1c1f221 100644 --- a/sound/soc/bcm/bcm2835-i2s.c +++ b/sound/soc/bcm/bcm2835-i2s.c
- dev->i2s_regmap = regmap[0];
- dev->clk_regmap = regmap[1];
- /* get the clock */
- dev->clk_prepared = false;
- dev->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(dev->clk)) {
dev_err(&pdev->dev, "could not get clk: %ld\n",PTR_ERR(dev->clk));return PTR_ERR(dev->clk);- }
- /* Request ioarea */
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(&pdev->dev, mem);
- if (IS_ERR(base))
return PTR_ERR(base);- dev->i2s_regmap = devm_regmap_init_mmio(&pdev->dev, base,
&bcm2835_regmap_config);- if (IS_ERR(dev->i2s_regmap))
return PTR_ERR(dev->i2s_regmap);- /* Set the DMA address - we have to parse DT ourselves */
- addr = of_get_address(pdev->dev.of_node, 0, NULL, NULL);
- if (!addr) {
dev_err(&pdev->dev, "could not get DMA-register address\n");return -EINVAL;- }
- dma_base = be32_to_cpup(addr);
Why aren't we just using mem->start like before? That seems like an independent change that should be justified on its own. I'd be ready to ack the patch if that change is removed.
Problem is that we need the VC4 bus-address (0x7e203000), which is the actual <reg> value from the device tree without any mapping.
Not the ARM MMU visible address mappings that mem->start provides (typically 0x20203000 or 0x3f203000 for bcm2836)
Nor the mapped address (base) available in the kernel (typically 0xdc......).
This is the only way to get it correctly and has been done the same way with spi-bcm2835.
Martin Sperl kernel@martin.sperl.org writes:
On 28.01.2016 23:08, Eric Anholt wrote:
kernel@martin.sperl.org writes:
From: Martin Sperl kernel@martin.sperl.org
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") this driver was no longer functional as it was manipulating the clock registers locally without going true the framework.
This patch moves to use the new clock framework and also moves away from the hardcoded address offsets for DMA getting the dma-address directly from the device tree.
Note that the optimal bclk_ratio selection to avoid jitter due to the use of fractional dividers, which is in the current version has been removed, because not all devices support these non power of 2 sized transfers, which resulted in lots of (downstream) modules that use: snd_soc_dai_set_bclk_ratio(cpu_dai, sample_bits * 2);
Signed-off-by: Martin Sperl kernel@martin.sperl.org
sound/soc/bcm/bcm2835-i2s.c | 284 ++++++++++--------------------------------- 1 file changed, 64 insertions(+), 220 deletions(-)
diff --git a/sound/soc/bcm/bcm2835-i2s.c b/sound/soc/bcm/bcm2835-i2s.c index 3303d5f..1c1f221 100644 --- a/sound/soc/bcm/bcm2835-i2s.c +++ b/sound/soc/bcm/bcm2835-i2s.c
- dev->i2s_regmap = regmap[0];
- dev->clk_regmap = regmap[1];
- /* get the clock */
- dev->clk_prepared = false;
- dev->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(dev->clk)) {
dev_err(&pdev->dev, "could not get clk: %ld\n",PTR_ERR(dev->clk));return PTR_ERR(dev->clk);- }
- /* Request ioarea */
- mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- base = devm_ioremap_resource(&pdev->dev, mem);
- if (IS_ERR(base))
return PTR_ERR(base);- dev->i2s_regmap = devm_regmap_init_mmio(&pdev->dev, base,
&bcm2835_regmap_config);- if (IS_ERR(dev->i2s_regmap))
return PTR_ERR(dev->i2s_regmap);- /* Set the DMA address - we have to parse DT ourselves */
- addr = of_get_address(pdev->dev.of_node, 0, NULL, NULL);
- if (!addr) {
dev_err(&pdev->dev, "could not get DMA-register address\n");return -EINVAL;- }
- dma_base = be32_to_cpup(addr);
Why aren't we just using mem->start like before? That seems like an independent change that should be justified on its own. I'd be ready to ack the patch if that change is removed.
Problem is that we need the VC4 bus-address (0x7e203000), which is the actual <reg> value from the device tree without any mapping.
Not the ARM MMU visible address mappings that mem->start provides (typically 0x20203000 or 0x3f203000 for bcm2836)
Nor the mapped address (base) available in the kernel (typically 0xdc......).
Now that I've noticed the BCM2835_VCMMU_SHIFT removal, this makes sense, but when you put unrelated changes like this together you end up slowing down the review process on your patches. Please separate it out into a separate commit.
On 13.02.2016, at 01:47, Eric Anholt eric@anholt.net wrote:
Martin Sperl kernel@martin.sperl.org writes:
Problem is that we need the VC4 bus-address (0x7e203000), which is the actual <reg> value from the device tree without any mapping.
Not the ARM MMU visible address mappings that mem->start provides (typically 0x20203000 or 0x3f203000 for bcm2836)
Nor the mapped address (base) available in the kernel (typically 0xdc......).
Now that I've noticed the BCM2835_VCMMU_SHIFT removal, this makes sense, but when you put unrelated changes like this together you end up slowing down the review process on your patches. Please separate it out into a separate commit.
OK - I agree: I really should have separated those, but as the driver was broken for both reasons and the MMU_SHIFT was the earliest fix I forgot about it when I got the final version for submission.
Anyway: Mark has already applied this patch on January 15th into for-next: https://git.kernel.org/cgit/linux/kernel/git/broonie/sound.git/commit/sound/... so it should go into the next release...
So unless Mark wants to revert it and have two separate patches instead, I guess we need to leave it like it is.
Martin
From: Martin Sperl kernel@martin.sperl.org
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") the bcm2835-i2s driver was no longer working.
This patch fixes the address ranges: * remove the PCM clock register range that is owned by the clockmanager * fix the length, which did not include the last register of this device
It also adds the required pcm-clock.
Signed-off-by: Martin Sperl kernel@martin.sperl.org --- arch/arm/boot/dts/bcm2835.dtsi | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi index aef64de..83d9787 100644 --- a/arch/arm/boot/dts/bcm2835.dtsi +++ b/arch/arm/boot/dts/bcm2835.dtsi @@ -120,9 +120,8 @@
i2s: i2s@7e203000 { compatible = "brcm,bcm2835-i2s"; - reg = <0x7e203000 0x20>, - <0x7e101098 0x02>; - + reg = <0x7e203000 0x24>; + clocks = <&clocks BCM2835_CLOCK_PCM>; dmas = <&dma 2>, <&dma 3>; dma-names = "tx", "rx";
Hi Martin,
[add Mike and Remi]
Am 12.01.2016 um 13:35 schrieb kernel@martin.sperl.org:
From: Martin Sperl kernel@martin.sperl.org
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") the bcm2835-i2s driver was no longer working.
This patch fixes the address ranges:
- remove the PCM clock register range that is owned by the clockmanager
- fix the length, which did not include the last register of this device
It also adds the required pcm-clock.
Signed-off-by: Martin Sperl kernel@martin.sperl.org
arch/arm/boot/dts/bcm2835.dtsi | 5 ++---
this won't apply, because the file has been renamed to bcm283x.dtsi.
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi index aef64de..83d9787 100644 --- a/arch/arm/boot/dts/bcm2835.dtsi +++ b/arch/arm/boot/dts/bcm2835.dtsi @@ -120,9 +120,8 @@
i2s: i2s@7e203000 { compatible = "brcm,bcm2835-i2s";
reg = <0x7e203000 0x20>,<0x7e101098 0x02>;
reg = <0x7e203000 0x24>;clocks = <&clocks BCM2835_CLOCK_PCM>;
After applying clk series ([PATCH V4 0/7] clk: bcm2835: add clocks and add MASH support) and this series the pcm clock is an orphan.
Do we need to add "assigned-clocks" to the i2s node just like for pwm [1]?
Regards Stefan
dmas = <&dma 2>, <&dma 3>; dma-names = "tx", "rx";
[1] - http://lists.infradead.org/pipermail/linux-rpi-kernel/2015-December/002789.h...
On 16.01.2016, at 16:26, Stefan Wahren info@lategoodbye.de wrote:
Hi Martin,
[add Mike and Remi]
Am 12.01.2016 um 13:35 schrieb kernel@martin.sperl.org:
From: Martin Sperl kernel@martin.sperl.org
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") the bcm2835-i2s driver was no longer working.
This patch fixes the address ranges:
- remove the PCM clock register range that is owned by the clockmanager
- fix the length, which did not include the last register of this device
It also adds the required pcm-clock.
Signed-off-by: Martin Sperl kernel@martin.sperl.org
arch/arm/boot/dts/bcm2835.dtsi | 5 ++---
this won't apply, because the file has been renamed to bcm283x.dtsi.
Well - this "rename” was and still is not merged upstream, so it is an unfortunate circumstance as I am going on vacation and can not create a new patchset until I return. So please take it as a template when applying it.
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi index aef64de..83d9787 100644 --- a/arch/arm/boot/dts/bcm2835.dtsi +++ b/arch/arm/boot/dts/bcm2835.dtsi @@ -120,9 +120,8 @@
i2s: i2s@7e203000 { compatible = "brcm,bcm2835-i2s";
reg = <0x7e203000 0x20>,<0x7e101098 0x02>;
reg = <0x7e203000 0x24>;clocks = <&clocks BCM2835_CLOCK_PCM>;After applying clk series ([PATCH V4 0/7] clk: bcm2835: add clocks and add MASH support) and this series the pcm clock is an orphan.
Do we need to add "assigned-clocks" to the i2s node just like for pwm [1]?
In my experience it is not needed for PCM, as the clock is set by the bcm2835-i2s driver, so I left it out.
As I do not know how the orphan PWM clock would be used I can not comment if this is really needed with PWM or not - it would just set the default clock if it got referenced in the DT.
Thanks, Martin
kernel@martin.sperl.org writes:
From: Martin Sperl kernel@martin.sperl.org
Since the move to the new clock framework with commit 94cb7f76caa0 ("ARM: bcm2835: Switch to using the new clock driver support.") the bcm2835-i2s driver was no longer working.
This patch fixes the address ranges:
- remove the PCM clock register range that is owned by the clockmanager
- fix the length, which did not include the last register of this device
It also adds the required pcm-clock.
Signed-off-by: Martin Sperl kernel@martin.sperl.org
Acked-by: Eric Anholt eric@anholt.net
From: Martin Sperl kernel@martin.sperl.org
The bcm2835-i2s driver has been updated to use the new clock framework for the bcm2835 SOC.
This patch documents the required changes to the bindings.
Signed-off-by: Martin Sperl kernel@martin.sperl.org --- Documentation/devicetree/bindings/sound/brcm,bcm2835-i2s.txt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/brcm,bcm2835-i2s.txt b/Documentation/devicetree/bindings/sound/brcm,bcm2835-i2s.txt index 65783de..b331f26 100644 --- a/Documentation/devicetree/bindings/sound/brcm,bcm2835-i2s.txt +++ b/Documentation/devicetree/bindings/sound/brcm,bcm2835-i2s.txt @@ -4,11 +4,10 @@ Required properties: - compatible: "brcm,bcm2835-i2s" - reg: A list of base address and size entries: * The first entry should cover the PCM registers - * The second entry should cover the PCM clock registers +- clocks: the (PCM) clock to use - dmas: List of DMA controller phandle and DMA request line ordered pairs. - dma-names: Identifier string for each DMA request line in the dmas property. These strings correspond 1:1 with the ordered pairs in dmas. - One of the DMA channels will be responsible for transmission (should be named "tx") and one for reception (should be named "rx").
@@ -16,8 +15,8 @@ Example:
bcm2835_i2s: i2s@7e203000 { compatible = "brcm,bcm2835-i2s"; - reg = <0x7e203000 0x20>, - <0x7e101098 0x02>; + reg = <0x7e203000 0x24>; + clocks = <&clocks BCM2835_CLOCK_PCM>;
dmas = <&dma 2>, <&dma 3>;
On Tue, Jan 12, 2016 at 12:35:48PM +0000, kernel@martin.sperl.org wrote:
From: Martin Sperl kernel@martin.sperl.org
Typo in the subject (bsm).
The bcm2835-i2s driver has been updated to use the new clock framework for the bcm2835 SOC.
This patch documents the required changes to the bindings.
Signed-off-by: Martin Sperl kernel@martin.sperl.org
Please add acks when you post new versions.
On 12.01.2016, at 15:36, Rob Herring robh@kernel.org wrote:
On Tue, Jan 12, 2016 at 12:35:48PM +0000, kernel@martin.sperl.org wrote:
From: Martin Sperl kernel@martin.sperl.org
Typo in the subject (bsm).
The bcm2835-i2s driver has been updated to use the new clock framework for the bcm2835 SOC.
This patch documents the required changes to the bindings.
Signed-off-by: Martin Sperl kernel@martin.sperl.org
Please add acks when you post new versions.
You want it resent with the typo fixed and signed-off?
Martin Sperl kernel@martin.sperl.org writes:
On 12.01.2016, at 15:36, Rob Herring robh@kernel.org wrote:
On Tue, Jan 12, 2016 at 12:35:48PM +0000, kernel@martin.sperl.org wrote:
From: Martin Sperl kernel@martin.sperl.org
Typo in the subject (bsm).
The bcm2835-i2s driver has been updated to use the new clock framework for the bcm2835 SOC.
This patch documents the required changes to the bindings.
Signed-off-by: Martin Sperl kernel@martin.sperl.org
Please add acks when you post new versions.
You want it resent with the typo fixed and signed-off?
Generally, make sure you apply acks as you get them, so that they don't get lost. If you fix the subject and add Rob's ack, it's also:
Acked-by: Eric Anholt eric@anholt.net
participants (6)
-
Eric Anholt -
kernel＠martin.sperl.org -
Mark Brown -
Martin Sperl
-
Rob Herring -
Stefan Wahren