[PATCH 0/6] ASoC: samsung: fsd: audio support for FSD SoC
The intention of this patch series is to enable audio support on FSD SoC.
The following features are added in samsung I2S interface: 1. Add TDM support on samsung I2S interface 2. Allow sound card to directly configure prescaler divider instead of calculating it from frame clock
Also the sound card support for FSD SoC is added, which utilizes samsung I2S interface as CPU DAI.
This patch is dependent on fsd-pinctrl fixes patch series [1] [1]: https://lkml.org/lkml/2022/10/13/257
Padmanabhan Rajanbabu (6): ASoC: samsung: i2s: TDM Support for CPU DAI driver ASoC: samsung: i2s: configure PSR from sound card dt-bindings: sound: Add sound card bindings for Tesla FSD ASoC: samsung: fsd: Add FSD soundcard driver arm64: dts: fsd: Add I2S DAI node for Tesla FSD arm64: dts: fsd: Add sound card node for Tesla FSD
.../bindings/sound/tesla,fsd-card.yaml | 158 ++++++++ arch/arm64/boot/dts/tesla/fsd-evb.dts | 57 +++ arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 14 + arch/arm64/boot/dts/tesla/fsd.dtsi | 41 ++ sound/soc/samsung/Kconfig | 12 + sound/soc/samsung/Makefile | 2 + sound/soc/samsung/fsd-card.c | 349 ++++++++++++++++++ sound/soc/samsung/i2s-regs.h | 17 + sound/soc/samsung/i2s.c | 120 +++++- sound/soc/samsung/i2s.h | 1 + 10 files changed, 766 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml create mode 100644 sound/soc/samsung/fsd-card.c
Add support to configure samsung I2S CPU DAI in TDM mode.
Signed-off-by: Chandrasekar R rcsekar@samsung.com Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com --- sound/soc/samsung/i2s-regs.h | 15 +++++++ sound/soc/samsung/i2s.c | 84 +++++++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/sound/soc/samsung/i2s-regs.h b/sound/soc/samsung/i2s-regs.h index b4b5d6053503..cb2be4a3b70b 100644 --- a/sound/soc/samsung/i2s-regs.h +++ b/sound/soc/samsung/i2s-regs.h @@ -154,4 +154,19 @@ #define I2SSIZE_TRNMSK (0xffff) #define I2SSIZE_SHIFT (16)
+#define TDM_LRCLK_WIDTH_SHIFT 12 +#define TDM_LRCLK_WIDTH_MASK 0xFF +#define TDM_RX_SLOTS_SHIFT 8 +#define TDM_RX_SLOTS_MASK 7 +#define TDM_TX_SLOTS_SHIFT 4 +#define TDM_TX_SLOTS_MASK 7 +#define TDM_MODE_MASK 1 +#define TDM_MODE_SHIFT 1 +#define TDM_MODE_DSPA 0 +#define TDM_MODE_DSPB 1 +#define TDM_ENABLE (1 << 0) + +/* stereo default */ +#define TDM_DEFAULT_SLOT_NUM_DIVIDER 2 + #endif /* __SND_SOC_SAMSUNG_I2S_REGS_H */ diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index 9505200f3d11..fb806b0af6ab 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -117,6 +117,8 @@ struct samsung_i2s_priv { struct clk *clk_table[3]; struct clk_onecell_data clk_data;
+ int tdm_slots; + /* Spinlock protecting member fields below */ spinlock_t lock;
@@ -625,15 +627,19 @@ static int i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) struct samsung_i2s_priv *priv = snd_soc_dai_get_drvdata(dai); struct i2s_dai *i2s = to_info(dai); int lrp_shift, sdf_shift, sdf_mask, lrp_rlow, mod_slave; + int tdm_mod_mask, tdm_mod_shift; + u32 tdm = 0, tdm_tmp = 0; u32 mod, tmp = 0; unsigned long flags;
lrp_shift = priv->variant_regs->lrp_off; sdf_shift = priv->variant_regs->sdf_off; + tdm_mod_shift = TDM_MODE_SHIFT; mod_slave = 1 << priv->variant_regs->mss_off;
sdf_mask = MOD_SDF_MASK << sdf_shift; lrp_rlow = MOD_LR_RLOW << lrp_shift; + tdm_mod_mask = TDM_MODE_MASK << tdm_mod_shift;
/* Format is priority */ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { @@ -648,6 +654,20 @@ static int i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) case SND_SOC_DAIFMT_I2S: tmp |= (MOD_SDF_IIS << sdf_shift); break; + case SND_SOC_DAIFMT_DSP_A: + if (!(priv->quirks & QUIRK_SUPPORTS_TDM)) { + dev_err(&i2s->pdev->dev, "TDM mode not supported\n"); + return -EINVAL; + } + tdm_tmp |= (TDM_MODE_DSPA << tdm_mod_shift); + break; + case SND_SOC_DAIFMT_DSP_B: + if (!(priv->quirks & QUIRK_SUPPORTS_TDM)) { + dev_err(&i2s->pdev->dev, "TDM mode not supported\n"); + return -EINVAL; + } + tdm_tmp |= (TDM_MODE_DSPB << tdm_mod_shift); + break; default: dev_err(&i2s->pdev->dev, "Format not supported\n"); return -EINVAL; @@ -693,12 +713,17 @@ static int i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) pm_runtime_get_sync(dai->dev); spin_lock_irqsave(&priv->lock, flags); mod = readl(priv->addr + I2SMOD); + + if (priv->quirks & QUIRK_SUPPORTS_TDM) + tdm = readl(priv->addr + I2STDM); /* * Don't change the I2S mode if any controller is active on this * channel. */ if (any_active(i2s) && - ((mod & (sdf_mask | lrp_rlow | mod_slave)) != tmp)) { + (((mod & (sdf_mask | lrp_rlow | mod_slave)) != tmp) || + ((priv->quirks & QUIRK_SUPPORTS_TDM) && + ((tdm & tdm_mod_mask) != tdm_tmp)))) { spin_unlock_irqrestore(&priv->lock, flags); pm_runtime_put(dai->dev); dev_err(&i2s->pdev->dev, @@ -706,6 +731,12 @@ static int i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) return -EAGAIN; }
+ if (priv->quirks & QUIRK_SUPPORTS_TDM) { + tdm &= ~(tdm_mod_mask); + tdm |= tdm_tmp; + writel(tdm, priv->addr + I2STDM); + } + mod &= ~(sdf_mask | lrp_rlow | mod_slave); mod |= tmp; writel(mod, priv->addr + I2SMOD); @@ -812,6 +843,47 @@ static int i2s_hw_params(struct snd_pcm_substream *substream, return 0; }
+static int i2s_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask, + unsigned int rx_mask, int slots, int slot_width) +{ + struct samsung_i2s_priv *priv = snd_soc_dai_get_drvdata(dai); + struct i2s_dai *i2s = to_info(dai); + u32 tdm = 0, mask = 0, val = 0; + unsigned long flags; + + if (!(priv->quirks & QUIRK_SUPPORTS_TDM)) { + dev_err(&i2s->pdev->dev, "Invalid request: TDM not enabled\n"); + return -EINVAL; + } + + mask |= (TDM_ENABLE); + mask |= (TDM_TX_SLOTS_MASK << TDM_TX_SLOTS_SHIFT); + mask |= (TDM_RX_SLOTS_MASK << TDM_RX_SLOTS_SHIFT); + + if (slots) { + val |= ((slots-1) & TDM_TX_SLOTS_MASK) << TDM_TX_SLOTS_SHIFT; + val |= ((slots-1) & TDM_RX_SLOTS_MASK) << TDM_RX_SLOTS_SHIFT; + + dev_info(&i2s->pdev->dev, + "TDM Mode Configured - TX and RX Slots: %d\n", slots); + + val |= TDM_ENABLE; + + priv->tdm_slots = slots; + } else { + val = 0; + priv->tdm_slots = 0; + } + + spin_lock_irqsave(&priv->lock, flags); + tdm = readl(priv->addr + I2STDM); + tdm = (tdm & ~mask) | val; + writel(tdm, priv->addr + I2STDM); + spin_unlock_irqrestore(&priv->lock, flags); + + return 0; +} + /* We set constraints on the substream according to the version of I2S */ static int i2s_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) @@ -879,6 +951,9 @@ static int config_setup(struct i2s_dai *i2s) if (!bfs && other) bfs = other->bfs;
+ if (!bfs && (priv->quirks & QUIRK_SUPPORTS_TDM) && priv->tdm_slots) + bfs = blc * priv->tdm_slots; + /* Select least possible multiple(2) if no constraint set */ if (!bfs) bfs = blc * 2; @@ -899,6 +974,9 @@ static int config_setup(struct i2s_dai *i2s) rfs = 256; else rfs = 384; + + if ((priv->quirks & QUIRK_SUPPORTS_TDM) && priv->tdm_slots) + rfs /= (priv->tdm_slots / TDM_DEFAULT_SLOT_NUM_DIVIDER); }
/* If already setup and running */ @@ -1110,6 +1188,7 @@ static const struct snd_soc_dai_ops samsung_i2s_dai_ops = { .set_fmt = i2s_set_fmt, .set_clkdiv = i2s_set_clkdiv, .set_sysclk = i2s_set_sysclk, + .set_tdm_slot = i2s_set_tdm_slot, .startup = i2s_startup, .shutdown = i2s_shutdown, .delay = i2s_delay, @@ -1464,6 +1543,9 @@ static int samsung_i2s_probe(struct platform_device *pdev) dev_err(&pdev->dev, "failed to enable clock: %d\n", ret); return ret; } + + priv->tdm_slots = 0; + pri_dai->dma_playback.addr = regs_base + I2STXD; pri_dai->dma_capture.addr = regs_base + I2SRXD; pri_dai->dma_playback.chan_name = "tx";
Currently the prescaler value in samsung I2S dai is calculated by dividing the peripheral input clock frequency with frame clock frequency and root clock frequency divider. This prescaler value is used to divide the input clock to generate root clock (RCLK) from which frame clock is generated for I2S communication.
However for the platforms which does not have a dedicated audio PLL as an input clock source, the prescaler divider will not generate accurate root clock frequency, which inturn affects sampling frequency also.
To overcome this scenario, support has been added to let the sound card identify right prescaler divider value and configure the prescaler (PSR) divider directly the from the sound card to achieve near accurate sample frequencies
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com --- sound/soc/samsung/i2s-regs.h | 2 ++ sound/soc/samsung/i2s.c | 36 ++++++++++++++++++++++++++++++++---- sound/soc/samsung/i2s.h | 1 + 3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/sound/soc/samsung/i2s-regs.h b/sound/soc/samsung/i2s-regs.h index cb2be4a3b70b..e2581dc73df2 100644 --- a/sound/soc/samsung/i2s-regs.h +++ b/sound/soc/samsung/i2s-regs.h @@ -132,6 +132,8 @@ #define EXYNOS7_MOD_RCLK_192FS 7
#define PSR_PSREN (1 << 15) +#define PSR_PSVAL_SHIFT 8 +#define PSR_PSVAL_MASK 0x3f
#define FIC_TX2COUNT(x) (((x) >> 24) & 0xf) #define FIC_TX1COUNT(x) (((x) >> 16) & 0xf) diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index fb806b0af6ab..a96286b27f1d 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -59,10 +59,10 @@ struct i2s_dai { /* Frame clock */ unsigned frmclk; /* - * Specifically requested RCLK, BCLK by machine driver. + * Specifically requested RCLK, BCLK and PSR by machine driver. * 0 indicates CPU driver is free to choose any value. */ - unsigned rfs, bfs; + unsigned int rfs, bfs, psr; /* Pointer to the Primary_Fifo if this is Sec_Fifo, NULL otherwise */ struct i2s_dai *pri_dai; /* Pointer to the Secondary_Fifo if it has one, NULL otherwise */ @@ -389,6 +389,17 @@ static inline int get_blc(struct i2s_dai *i2s) } }
+static inline unsigned int get_psval(struct i2s_dai *i2s) +{ + struct samsung_i2s_priv *priv = i2s->priv; + u32 psr; + + psr = readl(priv->addr + I2SPSR) >> PSR_PSVAL_SHIFT; + psr &= PSR_PSVAL_MASK; + + return (psr + 1); +} + /* TX channel control */ static void i2s_txctrl(struct i2s_dai *i2s, int on) { @@ -994,7 +1005,11 @@ static int config_setup(struct i2s_dai *i2s) return 0;
if (!(priv->quirks & QUIRK_NO_MUXPSR)) { - psr = priv->rclk_srcrate / i2s->frmclk / rfs; + if (i2s->psr) + psr = i2s->psr; + else + psr = priv->rclk_srcrate / i2s->frmclk / rfs; + writel(((psr - 1) << 8) | PSR_PSREN, priv->addr + I2SPSR); dev_dbg(&i2s->pdev->dev, "RCLK_SRC=%luHz PSR=%u, RCLK=%dfs, BCLK=%dfs\n", @@ -1072,6 +1087,18 @@ static int i2s_set_clkdiv(struct snd_soc_dai *dai, i2s->bfs = div; pm_runtime_put(dai->dev); break; + case SAMSUNG_I2S_DIV_RCLK: + pm_runtime_get_sync(dai->dev); + if ((any_active(i2s) && div && (get_psval(i2s) != div)) + || (other && other->psr && (other->psr != div))) { + pm_runtime_put(dai->dev); + dev_err(&i2s->pdev->dev, + "%s:%d Other DAI busy\n", __func__, __LINE__); + return -EAGAIN; + } + i2s->psr = div; + pm_runtime_put(dai->dev); + break; default: dev_err(&i2s->pdev->dev, "Invalid clock divider(%d)\n", div_id); @@ -1140,9 +1167,10 @@ static int samsung_i2s_dai_probe(struct snd_soc_dai *dai) other->idma_playback.addr); }
- /* Reset any constraint on RFS and BFS */ + /* Reset any constraint on RFS, BFS and PSR*/ i2s->rfs = 0; i2s->bfs = 0; + i2s->psr = 0;
spin_lock_irqsave(&priv->lock, flags); i2s_txctrl(i2s, 0); diff --git a/sound/soc/samsung/i2s.h b/sound/soc/samsung/i2s.h index 78b475ef98d9..e783d33fdfac 100644 --- a/sound/soc/samsung/i2s.h +++ b/sound/soc/samsung/i2s.h @@ -13,6 +13,7 @@ #define SAMSUNG_I2S_DAI_SEC "samsung-i2s-sec"
#define SAMSUNG_I2S_DIV_BCLK 1 +#define SAMSUNG_I2S_DIV_RCLK 2
#define SAMSUNG_I2S_RCLKSRC_0 0 #define SAMSUNG_I2S_RCLKSRC_1 1
On Fri, Oct 14, 2022 at 03:51:47PM +0530, Padmanabhan Rajanbabu wrote:
Currently the prescaler value in samsung I2S dai is calculated by dividing the peripheral input clock frequency with frame clock frequency and root clock frequency divider. This prescaler value is used to divide the input clock to generate root clock (RCLK) from which frame clock is generated for I2S communication.
However for the platforms which does not have a dedicated audio PLL as an input clock source, the prescaler divider will not generate accurate root clock frequency, which inturn affects sampling frequency also.
To overcome this scenario, support has been added to let the sound card identify right prescaler divider value and configure the prescaler (PSR) divider directly the from the sound card to achieve near accurate sample frequencies
It's not clear to me why the solution here is to move the configuration to the sound card rather than to improve the I2S driver to be able to cope with whatever the restrictions are on the PSR in these systems - it seems more cumbersome for system integrators, especially since you've not documented the issues or how to configure it. Could you expand on what the constraints are here and why it's not possible for the driver to figure things out (given some quirk information)?
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: 14 October 2022 05:32 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com Cc: lgirdwood@gmail.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com; alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung- soc@vger.kernel.org Subject: Re: [PATCH 2/6] ASoC: samsung: i2s: configure PSR from sound card
On Fri, Oct 14, 2022 at 03:51:47PM +0530, Padmanabhan Rajanbabu wrote:
Currently the prescaler value in samsung I2S dai is calculated by dividing the peripheral input clock frequency with frame clock frequency and root clock frequency divider. This prescaler value is used to divide the input clock to generate root clock (RCLK) from which frame clock is generated for I2S communication.
However for the platforms which does not have a dedicated audio PLL as an input clock source, the prescaler divider will not generate accurate root clock frequency, which inturn affects sampling frequency
also.
To overcome this scenario, support has been added to let the sound card identify right prescaler divider value and configure the prescaler (PSR) divider directly the from the sound card to achieve near accurate sample frequencies
It's not clear to me why the solution here is to move the configuration to
the
sound card rather than to improve the I2S driver to be able to cope with whatever the restrictions are on the PSR in these systems - it seems more cumbersome for system integrators, especially since you've not documented the issues or how to configure it. Could you expand on what the
constraints
are here and why it's not possible for the driver to figure things out
(given
some quirk information)?
Thank you for reviewing the patch.
In Samsung I2S CPU controller, to derive the frame clock, we are supposed to configure the PSR and RFS internal dividers. i.e.
OPCLK -> PSR -> RCLK -> RFS -> Frame clock
Where: OPCLK - Operational clock PSR - Operational clock prescaler RCLK - Root Clock (derived from OPCLK based on PSR) RFS - Root frequency selection (divider) Frame clock - Sample frequency (derived from RCLK based on RFS)
Ultimately,
PSR = OPCLK / Frame clock / RFS
Unlike other platforms utilizing Samsung CPU DAI, FSD SoC has a limitation on operational clock, where the clock frequency is fixed (66 MHz) and cannot be modified.
Assuming that an userspace application wants perform playback @44100 Hz and assuming that RFS divider value is configured as 256, the PSR value will yield to
66 MHz / 44.1 KHz / 256 = 5
However if HW uses PSR = 5 to derive the frame clock from operational clock, then
RCLK = OPCLK / PSR = 66 MHz / 5 = 13.2 MHz Frame clock = RCLK / RFS = 13.2 MHz / 256 = 51562 Hz
The actual frame clock derived based on PSR is now different from what user application has intended. The situation did not improve even if the RFS is swept throughout the entire valid range.
We can overcome this scenario to an extent if we can get a flexibility to Configure both PSR as well as RFS.
i.e. to achieve frame clock of 44100 Hz, if PSR = 23 and RFS = 64 then frame clock = 66 MHz / 23 / 64 = 44836 Hz
Although the sample frequency is not precise, it is very much closer to the Intended frequency, when compared to that of the existing solution. Since this scenario is specific to FSD SoC and has no changes in the Samsung I2S CPU DAI, the configuration is being done from the sound card of FSD SoC during hw_params.
Please let me know if you think this scenario can be approached in any other way possible, rather than configuring from sound card.
On Fri, Oct 21, 2022 at 01:30:25PM +0530, Padmanabhan Rajanbabu wrote:
We can overcome this scenario to an extent if we can get a flexibility to Configure both PSR as well as RFS.
Why does it make sense for the machine driver to worry about this rather than having the I2S controller driver configure the clock tree?
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: 21 October 2022 05:24 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com Cc: lgirdwood@gmail.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com; alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung- soc@vger.kernel.org Subject: Re: [PATCH 2/6] ASoC: samsung: i2s: configure PSR from sound card
On Fri, Oct 21, 2022 at 01:30:25PM +0530, Padmanabhan Rajanbabu wrote:
We can overcome this scenario to an extent if we can get a flexibility to Configure both PSR as well as RFS.
Why does it make sense for the machine driver to worry about this rather than having the I2S controller driver configure the clock tree?
____________________________________________ _____ | __ | | | | | \ | |CMU| | | \ | |FSD |- |---|-|--------->| \ _________ _________ | |___ | | | |op_clk0| | | | | | | | | | |MUX|----| PSR |----| RFS |--cdclk | | | | | | |_______| |_______| | | | |--------->| / | | | op_clk1 | / | | | |_ / | | |___________________________________________| | |-----> To other FSD SoC Peripherals In FSD I2S, the clock source is not an independent source but a common clock source being shared by many IPs in the same domain.
Changing the clock tree will impact other IPs in the domain as they are dependent on the same source for functionality.
We can understand your point to bring the PSR changes under the I2S CPU DAI driver by adding a separate compatible and data for the FSD SoC. But If we take the example of existing sound cards such as sound/soc/samsung/tm2_wm5110.c, the op_clk is supplied via external audio pll to the controller and PLL configuration is taken care by the sound card. Since the configuration of PLL is more specific to the tm2 platform, it makes use of the flexibility of changing the RFS and BFS using the sysclk and clkdiv hooks provided by exynos7-i2s CPU DAI along with PLL tuning for precise sampling frequency.
Similar to the above example, the choice of clock source under discussion is not a limitation of exynos7-i2s controller, but instead is a limitation on the FSD SoC. By using the proposed change, we can ensure that the exynos CPU DAI driver is giving additional hooks similar to existing hooks for BFS, RFS and CDCLK direction so that sound cards can use snd_soc_dai_set_sysclk and snd_soc_dai_set_clkdiv to customize the same.
An alternative approach is to use the cpu dai as bit clock and frame clock consumer (i.e. in slave mode) so that codec can supply the MCLK to FSD for playback and capture. But this will deviate from the actual usecase for FSD SoC, where the CPU DAI is intended to function as master.
On Tue, Nov 08, 2022 at 10:53:40AM +0530, Padmanabhan Rajanbabu wrote:
We can overcome this scenario to an extent if we can get a flexibility to Configure both PSR as well as RFS.
Why does it make sense for the machine driver to worry about this rather than having the I2S controller driver configure the clock tree?
_____ | __ | | | | | \ | |CMU| | | \ | |FSD |- |---|-|--------->| \ _________ _________ | |___ | | | |op_clk0| | | | | | | | | | |MUX|----| PSR |----| RFS |--cdclk | | | | | | |_______| |_______| | | | |--------->| / | | | op_clk1 | / | | | |_ / | | |___________________________________________| | |-----> To other FSD SoC Peripherals
In FSD I2S, the clock source is not an independent source but a common clock source being shared by many IPs in the same domain.
Changing the clock tree will impact other IPs in the domain as they are dependent on the same source for functionality.
I'm not sure I follow. Perhaps your diagram is unclear but it looks like PSR and RFS are both after a mux which appears to select which clock is going to be used by the I2S controller? The usage by other clocks appears to be upstream of the mux and dividers.
We can understand your point to bring the PSR changes under the I2S CPU DAI driver by adding a separate compatible and data for the FSD SoC. But If we take the example of existing sound cards such as sound/soc/samsung/tm2_wm5110.c, the op_clk is supplied via external audio pll to the controller and PLL configuration is taken care by the sound card. Since the configuration of PLL is more specific to the tm2 platform, it makes use of the flexibility of changing the RFS and BFS using the sysclk and clkdiv hooks provided by exynos7-i2s CPU DAI along with PLL tuning for precise sampling frequency.
The big reason for the clocking control (and indeed having a custom machine driver) with the WM5110 is that it has multiple clocks to control and a good deal of flexibility with placing them in clock domains and so on which have power and performance impacts. It's frankly a bit unclear to me if the CPU I2S controller even needs the bitclock configuring given that the clocks are being driven by the CODEC there, but regardless it's not clear to me why the I2S controller would need anything other than the input clock to the block configuring?
Similar to the above example, the choice of clock source under discussion is not a limitation of exynos7-i2s controller, but instead is a limitation on the FSD SoC. By using the proposed change, we can ensure that the exynos CPU DAI driver is giving additional hooks similar to existing hooks for BFS, RFS and CDCLK direction so that sound cards can use snd_soc_dai_set_sysclk and snd_soc_dai_set_clkdiv to customize the same.
I'm still not seeing anything that articulates why pushing the configuration of the dividers within the block into the machine driver solves a problem here. Again, what's the upside to configuring clocks that are purely within the block?
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: 09 November 2022 11:09 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com Cc: lgirdwood@gmail.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com; alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung- soc@vger.kernel.org Subject: Re: [PATCH 2/6] ASoC: samsung: i2s: configure PSR from sound card
On Tue, Nov 08, 2022 at 10:53:40AM +0530, Padmanabhan Rajanbabu wrote:
We can overcome this scenario to an extent if we can get a flexibility to Configure both PSR as well as RFS.
Why does it make sense for the machine driver to worry about this rather than having the I2S controller driver configure the clock tree?
_____ | __ | | | | | \ | |CMU| | | \ | |FSD |- |---|-|--------->| \ _________ _________ | |___ | | | |op_clk0| | | | | | | | | | |MUX|----| PSR |----| RFS |--cdclk | | | | | | |_______| |_______| | | | |--------->| / | | | op_clk1 | / | | | |_ / | | |___________________________________________| | |-----> To other FSD SoC Peripherals
In FSD I2S, the clock source is not an independent source but a common clock source being shared by many IPs in the same domain.
Changing the clock tree will impact other IPs in the domain as they are dependent on the same source for functionality.
I'm not sure I follow. Perhaps your diagram is unclear but it looks like
PSR and
RFS are both after a mux which appears to select which clock is going to
be
used by the I2S controller? The usage by other clocks appears to be upstream of the mux and dividers.
We can understand your point to bring the PSR changes under the I2S CPU DAI driver by adding a separate compatible and data for the FSD SoC. But If we take the example of existing sound cards such as sound/soc/samsung/tm2_wm5110.c, the op_clk is supplied via external audio pll to the controller and PLL configuration is taken care by the sound card. Since the configuration of PLL is more specific to the tm2 platform, it makes use of the flexibility of changing the RFS and BFS using the sysclk and clkdiv hooks provided by exynos7-i2s CPU DAI along with PLL tuning for precise sampling frequency.
The big reason for the clocking control (and indeed having a custom
machine
driver) with the WM5110 is that it has multiple clocks to control and a
good
deal of flexibility with placing them in clock domains and so on which
have
power and performance impacts. It's frankly a bit unclear to me if the
CPU
I2S controller even needs the bitclock configuring given that the clocks
are
being driven by the CODEC there, but regardless it's not clear to me why
the
I2S controller would need anything other than the input clock to the block configuring?
Similar to the above example, the choice of clock source under discussion is not a limitation of exynos7-i2s controller, but instead is a limitation on the FSD SoC. By using the proposed change, we can ensure that the exynos CPU DAI driver is giving additional hooks similar to existing hooks for BFS, RFS and CDCLK direction so that sound cards can use snd_soc_dai_set_sysclk and snd_soc_dai_set_clkdiv to customize the same.
I'm still not seeing anything that articulates why pushing the
configuration of
the dividers within the block into the machine driver solves a problem
here.
Again, what's the upside to configuring clocks that are purely within the block?
Okay, I can understand the reason for de-linking these changes from the machine Driver. I'll post the v2 patches integrating the PSR changes into cpu dai driver.
Thanks, Padmanabhan R.
On 21/10/2022 04:00, Padmanabhan Rajanbabu wrote:
It's not clear to me why the solution here is to move the configuration to
the
sound card rather than to improve the I2S driver to be able to cope with whatever the restrictions are on the PSR in these systems - it seems more cumbersome for system integrators, especially since you've not documented the issues or how to configure it. Could you expand on what the
constraints
are here and why it's not possible for the driver to figure things out
(given
some quirk information)?
Thank you for reviewing the patch.
In Samsung I2S CPU controller, to derive the frame clock, we are supposed to configure the PSR and RFS internal dividers. i.e.
OPCLK -> PSR -> RCLK -> RFS -> Frame clock
Where: OPCLK - Operational clock PSR - Operational clock prescaler RCLK - Root Clock (derived from OPCLK based on PSR) RFS - Root frequency selection (divider) Frame clock - Sample frequency (derived from RCLK based on RFS)
Ultimately,
PSR = OPCLK / Frame clock / RFS
Unlike other platforms utilizing Samsung CPU DAI, FSD SoC has a limitation on operational clock, where the clock frequency is fixed (66 MHz) and cannot be modified.
Assuming that an userspace application wants perform playback @44100 Hz and assuming that RFS divider value is configured as 256, the PSR value will yield to
66 MHz / 44.1 KHz / 256 = 5
However if HW uses PSR = 5 to derive the frame clock from operational clock, then
RCLK = OPCLK / PSR = 66 MHz / 5 = 13.2 MHz Frame clock = RCLK / RFS = 13.2 MHz / 256 = 51562 Hz
The actual frame clock derived based on PSR is now different from what user application has intended. The situation did not improve even if the RFS is swept throughout the entire valid range.
We can overcome this scenario to an extent if we can get a flexibility to Configure both PSR as well as RFS.
i.e. to achieve frame clock of 44100 Hz, if PSR = 23 and RFS = 64 then frame clock = 66 MHz / 23 / 64 = 44836 Hz
Although the sample frequency is not precise, it is very much closer to the Intended frequency, when compared to that of the existing solution. Since this scenario is specific to FSD SoC and has no changes in the Samsung I2S CPU DAI, the configuration is being done from the sound card of FSD SoC during hw_params.
Please let me know if you think this scenario can be approached in any other way possible, rather than configuring from sound card.
Entire new driver for this, instead of improving existing Samsung drivers... no, it is no the way. If you followed this approach you would send 20 drivers for each "specific" quirk difference.
Best regards, Krzysztof
Add dt-binding reference document to configure the DAI link for Tesla FSD sound card driver.
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com --- .../bindings/sound/tesla,fsd-card.yaml | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
diff --git a/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml new file mode 100644 index 000000000000..4bd590f4ee27 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml @@ -0,0 +1,158 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright 2022 Samsung Electronics Co. Ltd. +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/tesla,fsd-card.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Tesla FSD ASoC sound card driver + +maintainers: + - Padmanabhan Rajanbabu p.rajanbabu@samsung.com + +description: | + This binding describes the node properties and essential DT entries of FSD + SoC sound card node + +select: false + +properties: + compatible: + enum: + - tesla,fsd-sndcard + + model: + description: Describes the Name of the sound card + $ref: /schemas/types.yaml#/definitions/string + + widgets: + description: A list of DAPM widgets in the sound card. Each entry is a pair + of strings, the first being the widget name and the second being the + widget alias + $ref: /schemas/types.yaml#/definitions/string-array + + audio-routing: + description: A list of the connections between audio components. Each entry + is a pair of strings, the first being the connection's sink, the second + being the connection's source + $ref: /schemas/types.yaml#/definitions/string-array + + dai-tdm-slot-num: + description: Enables TDM mode and specifies the number of TDM slots to be + enabled + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2, 3, 4, 5, 6, 7, 8] + default: 2 + + dai-tdm-slot-width: + description: Specifies the slot width of each TDm slot enabled + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [8, 16, 24] + default: 16 + + dai-link: + description: Holds the DAI link data between CPU, Codec and Platform + type: object + properties: + link-name: + description: Specifies the name of the DAI link + $ref: /schemas/types.yaml#/definitions/string + + dai-format: + description: Specifies the serial data format of CPU DAI + $ref: /schemas/types.yaml#/definitions/string + + tesla,bitclock-master: + description: Specifies the phandle of bitclock master DAI + $ref: /schemas/types.yaml#/definitions/phandle + + tesla,frame-master: + description: Specifies the phandle of frameclock master DAI + $ref: /schemas/types.yaml#/definitions/phandle + + cpu: + description: Holds the CPU DAI node and instance + type: object + properties: + sound-dai: + description: Specifies the phandle of CPU DAI node + $ref: /schemas/types.yaml#/definitions/phandle-array + + required: + - sound-dai + + codec: + description: Holds the Codec DAI node and instance + type: object + properties: + sound-dai: + description: Specifies the phandle of Codec DAI node + $ref: /schemas/types.yaml#/definitions/phandle-array + + required: + - sound-dai + + required: + - link-name + - dai-format + - tesla,bitclock-master + - tesla,frame-master + - cpu + +dependencies: + dai-tdm-slot-width: [ 'dai-tdm-slot-num' ] + +required: + - compatible + - model + - dai-link + +additionalProperties: false + +examples: + - | + sound { + compatible = "tesla,fsd-sndcard"; + status = "disabled"; + model = "fsd-i2s"; + + primary-dai-link-0 { + link-name = "fsd-primary-0"; + dai-format = "i2s"; + tesla,bitclock-master = <&tdm_0>; + tesla,frame-master = <&tdm_0>; + cpu { + sound-dai = <&tdm_0 0>; + }; + }; + + secondary-dai-link-0 { + link-name = "fsd-secondary-0"; + dai-format = "i2s"; + tesla,bitclock-master = <&tdm_0>; + tesla,frame-master = <&tdm_0>; + cpu { + sound-dai = <&tdm_0 1>; + }; + }; + + primary-dai-link-1 { + link-name = "fsd-primary-1"; + dai-format = "i2s"; + tesla,bitclock-master = <&tdm_1>; + tesla,frame-master = <&tdm_1>; + cpu { + sound-dai = <&tdm_1 0>; + }; + }; + + secondary-dai-link-1 { + link-name = "fsd-secondary-1"; + dai-format = "i2s"; + tesla,bitclock-master = <&tdm_1>; + tesla,frame-master = <&tdm_1>; + cpu { + sound-dai = <&tdm_1 1>; + }; + }; + };
On Fri, Oct 14, 2022 at 03:51:48PM +0530, Padmanabhan Rajanbabu wrote:
Add dt-binding reference document to configure the DAI link for Tesla FSD sound card driver.
The simple-card or graph-card bindings don't work for you?
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
.../bindings/sound/tesla,fsd-card.yaml | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
diff --git a/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml new file mode 100644 index 000000000000..4bd590f4ee27 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml @@ -0,0 +1,158 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright 2022 Samsung Electronics Co. Ltd. +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/tesla,fsd-card.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: Tesla FSD ASoC sound card driver
+maintainers:
- Padmanabhan Rajanbabu p.rajanbabu@samsung.com
+description: |
- This binding describes the node properties and essential DT entries of FSD
- SoC sound card node
+select: false
Never apply this schema. Why?
+properties:
- compatible:
- enum:
- tesla,fsd-sndcard- model:
- description: Describes the Name of the sound card
- $ref: /schemas/types.yaml#/definitions/string
- widgets:
- description: A list of DAPM widgets in the sound card. Each entry is a pair
of strings, the first being the widget name and the second being thewidget alias- $ref: /schemas/types.yaml#/definitions/string-array
- audio-routing:
- description: A list of the connections between audio components. Each entry
is a pair of strings, the first being the connection's sink, the secondbeing the connection's source- $ref: /schemas/types.yaml#/definitions/string-array
- dai-tdm-slot-num:
- description: Enables TDM mode and specifies the number of TDM slots to be
enabled- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [0, 1, 2, 3, 4, 5, 6, 7, 8]
maximum: 8
- default: 2
- dai-tdm-slot-width:
- description: Specifies the slot width of each TDm slot enabled
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [8, 16, 24]
- default: 16
All the above have types defined. You should not be redefining the types.
- dai-link:
- description: Holds the DAI link data between CPU, Codec and Platform
- type: object
additionalProperties: false
- properties:
link-name:description: Specifies the name of the DAI link$ref: /schemas/types.yaml#/definitions/stringdai-format:description: Specifies the serial data format of CPU DAI$ref: /schemas/types.yaml#/definitions/stringtesla,bitclock-master:description: Specifies the phandle of bitclock master DAI$ref: /schemas/types.yaml#/definitions/phandletesla,frame-master:description: Specifies the phandle of frameclock master DAI$ref: /schemas/types.yaml#/definitions/phandle
These are common properties. Drop 'tesla'.
cpu:description: Holds the CPU DAI node and instancetype: object
additionalProperties: false
properties:sound-dai:description: Specifies the phandle of CPU DAI node$ref: /schemas/types.yaml#/definitions/phandle-arrayrequired:- sound-daicodec:description: Holds the Codec DAI node and instancetype: object
additionalProperties: false
properties:sound-dai:description: Specifies the phandle of Codec DAI node$ref: /schemas/types.yaml#/definitions/phandle-array
Already has a type. Need to define how many entries (maxItems: 1 ?).
required:- sound-dai- required:
- link-name- dai-format- tesla,bitclock-master- tesla,frame-master- cpu+dependencies:
- dai-tdm-slot-width: [ 'dai-tdm-slot-num' ]
This should be captured with tdm-slot.txt converted to schema.
+required:
- compatible
- model
- dai-link
+additionalProperties: false
+examples:
- |
- sound {
compatible = "tesla,fsd-sndcard";status = "disabled";
Why have a disabled example? Other than your example won't pass your schema.
model = "fsd-i2s";primary-dai-link-0 {link-name = "fsd-primary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 0>;};};secondary-dai-link-0 {link-name = "fsd-secondary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 1>;};};primary-dai-link-1 {link-name = "fsd-primary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 0>;};};secondary-dai-link-1 {link-name = "fsd-secondary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 1>;};};- };
-- 2.17.1
-----Original Message----- From: Rob Herring [mailto:robh@kernel.org] Sent: 14 October 2022 08:43 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com Cc: lgirdwood@gmail.com; broonie@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com; alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung- soc@vger.kernel.org Subject: Re: [PATCH 3/6] dt-bindings: sound: Add sound card bindings for Tesla FSD
On Fri, Oct 14, 2022 at 03:51:48PM +0530, Padmanabhan Rajanbabu wrote:
Add dt-binding reference document to configure the DAI link for Tesla FSD sound card driver.
The simple-card or graph-card bindings don't work for you?
Thank you for reviewing the patch.
The actual reason for having a custom sound card driver lies in the fact that the Samsung i2s cpu dai requires configuration of some Samsung specific properties like rfs, bfs, codec clock direction and root clock source. We do not have flexibility of configuring the same in simple card driver (sound/soc/generic/simple-card.c) or audio graph card driver (sound/soc/generic/audio-graph-card.c). The binding has been added to support the custom sound card driver.
I understand from your query that the newly added card has device tree nodes and properties which are used in simple card as well, but having a different or new prefixes. I believe to address that, we can restructure the string names to generic ones. But I would like to understand, to reuse the simple card or audio graph card bindings, can we add secondary compatible strings in the simple card dt-binding for the custom sound card to probe ?
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
.../bindings/sound/tesla,fsd-card.yaml | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
diff --git a/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml new file mode 100644 index 000000000000..4bd590f4ee27 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml @@ -0,0 +1,158 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright +2022 Samsung Electronics Co. Ltd. +%YAML 1.2 +--- +$id: +https://protect2.fireeye.com/v1/url?k=4ae54403-157e7d1c-4ae4cf4c-
000b
+abdfecba-9eb398ea304f8ae8&q=1&e=4935bed0-ce62-47dd-8faf-
4750b01e22d3&
+u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fsound%2Ftesla%2Cfsd- card.ya
+ml%23 +$schema: +https://protect2.fireeye.com/v1/url?k=8c72226e-d3e91b71-8c73a921-
000b
+abdfecba-3ce999f6c052255b&q=1&e=4935bed0-ce62-47dd-8faf-
4750b01e22d3&
+u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
+title: Tesla FSD ASoC sound card driver
+maintainers:
- Padmanabhan Rajanbabu p.rajanbabu@samsung.com
+description: |
- This binding describes the node properties and essential DT entries
+of FSD
- SoC sound card node
+select: false
Never apply this schema. Why?
Sorry about it, let me change the select property to true in the next patchset
+properties:
- compatible:
- enum:
- tesla,fsd-sndcard- model:
- description: Describes the Name of the sound card
- $ref: /schemas/types.yaml#/definitions/string
- widgets:
- description: A list of DAPM widgets in the sound card. Each entry
is a pair
of strings, the first being the widget name and the second being
the
widget alias- $ref: /schemas/types.yaml#/definitions/string-array
- audio-routing:
- description: A list of the connections between audio components.
Each entry
is a pair of strings, the first being the connection's sink, the
second
being the connection's source- $ref: /schemas/types.yaml#/definitions/string-array
- dai-tdm-slot-num:
- description: Enables TDM mode and specifies the number of TDM slots
to be
enabled- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [0, 1, 2, 3, 4, 5, 6, 7, 8]
maximum: 8
Okay
- default: 2
- dai-tdm-slot-width:
- description: Specifies the slot width of each TDm slot enabled
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [8, 16, 24]
- default: 16
All the above have types defined. You should not be redefining the types.
Okay
- dai-link:
- description: Holds the DAI link data between CPU, Codec and
Platform
- type: object
additionalProperties: false
okay
- properties:
link-name:description: Specifies the name of the DAI link$ref: /schemas/types.yaml#/definitions/stringdai-format:description: Specifies the serial data format of CPU DAI$ref: /schemas/types.yaml#/definitions/stringtesla,bitclock-master:description: Specifies the phandle of bitclock master DAI$ref: /schemas/types.yaml#/definitions/phandletesla,frame-master:description: Specifies the phandle of frameclock master DAI$ref: /schemas/types.yaml#/definitions/phandleThese are common properties. Drop 'tesla'.
Okay
cpu:description: Holds the CPU DAI node and instancetype: objectadditionalProperties: false
Okay
properties:sound-dai:description: Specifies the phandle of CPU DAI node$ref: /schemas/types.yaml#/definitions/phandle-arrayrequired:- sound-daicodec:description: Holds the Codec DAI node and instancetype: objectadditionalProperties: false
Okay
properties:sound-dai:description: Specifies the phandle of Codec DAI node$ref: /schemas/types.yaml#/definitions/phandle-arrayAlready has a type. Need to define how many entries (maxItems: 1 ?).
Okay. I'll update in the upcoming patch set
required:- sound-dai- required:
- link-name- dai-format- tesla,bitclock-master- tesla,frame-master- cpu+dependencies:
- dai-tdm-slot-width: [ 'dai-tdm-slot-num' ]
This should be captured with tdm-slot.txt converted to schema.
Okay
+required:
- compatible
- model
- dai-link
+additionalProperties: false
+examples:
- |
- sound {
compatible = "tesla,fsd-sndcard";status = "disabled";Why have a disabled example? Other than your example won't pass your schema.
Thanks for noticing, I'll double check and change the example keeping the status as enabled
model = "fsd-i2s";primary-dai-link-0 {link-name = "fsd-primary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 0>;};};secondary-dai-link-0 {link-name = "fsd-secondary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 1>;};};primary-dai-link-1 {link-name = "fsd-primary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 0>;};};secondary-dai-link-1 {link-name = "fsd-secondary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 1>;};};- };
-- 2.17.1
On 21/10/2022 04:44, Padmanabhan Rajanbabu wrote:
On Fri, Oct 14, 2022 at 03:51:48PM +0530, Padmanabhan Rajanbabu wrote:
Add dt-binding reference document to configure the DAI link for Tesla FSD sound card driver.
The simple-card or graph-card bindings don't work for you?
Thank you for reviewing the patch.
The actual reason for having a custom sound card driver lies in the fact that the Samsung i2s cpu dai requires configuration of some Samsung specific properties like rfs, bfs, codec clock direction and root clock source. We do not have flexibility of configuring the same in simple card driver (sound/soc/generic/simple-card.c) or audio graph card driver (sound/soc/generic/audio-graph-card.c). The binding has been added to support the custom sound card driver.
I understand from your query that the newly added card has device tree nodes and properties which are used in simple card as well, but having a different or new prefixes. I believe to address that, we can restructure the string names to generic ones.
You must use generic, existing properties where applicable.
But I would like to understand, to reuse the simple card or audio graph card bindings, can we add secondary compatible strings in the simple card dt-binding for the custom sound card to probe ?
If you see other vendor compatibles there, then yes... But there aren't any, right?
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
.../bindings/sound/tesla,fsd-card.yaml | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
diff --git a/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml new file mode 100644 index 000000000000..4bd590f4ee27 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml @@ -0,0 +1,158 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright +2022 Samsung Electronics Co. Ltd. +%YAML 1.2 +--- +$id: +https://protect2.fireeye.com/v1/url?k=4ae54403-157e7d1c-4ae4cf4c-
000b
+abdfecba-9eb398ea304f8ae8&q=1&e=4935bed0-ce62-47dd-8faf-
4750b01e22d3&
+u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fsound%2Ftesla%2Cfsd- card.ya
+ml%23 +$schema: +https://protect2.fireeye.com/v1/url?k=8c72226e-d3e91b71-8c73a921-
000b
+abdfecba-3ce999f6c052255b&q=1&e=4935bed0-ce62-47dd-8faf-
4750b01e22d3&
+u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
+title: Tesla FSD ASoC sound card driver
+maintainers:
- Padmanabhan Rajanbabu p.rajanbabu@samsung.com
+description: |
- This binding describes the node properties and essential DT entries
+of FSD
- SoC sound card node
+select: false
Never apply this schema. Why?
Sorry about it, let me change the select property to true in the next patchset
No, just drop it. Look at other bindings or at example-schema
+properties:
- compatible:
- enum:
- tesla,fsd-sndcard- model:
- description: Describes the Name of the sound card
- $ref: /schemas/types.yaml#/definitions/string
- widgets:
- description: A list of DAPM widgets in the sound card. Each entry
is a pair
of strings, the first being the widget name and the second beingthe
widget alias- $ref: /schemas/types.yaml#/definitions/string-array
- audio-routing:
- description: A list of the connections between audio components.
Each entry
is a pair of strings, the first being the connection's sink, thesecond
being the connection's source- $ref: /schemas/types.yaml#/definitions/string-array
- dai-tdm-slot-num:
- description: Enables TDM mode and specifies the number of TDM slots
to be
enabled- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [0, 1, 2, 3, 4, 5, 6, 7, 8]
maximum: 8
Okay
- default: 2
- dai-tdm-slot-width:
- description: Specifies the slot width of each TDm slot enabled
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [8, 16, 24]
- default: 16
All the above have types defined. You should not be redefining the types.
Okay
- dai-link:
- description: Holds the DAI link data between CPU, Codec and
Platform
- type: object
additionalProperties: falseokay
- properties:
link-name:description: Specifies the name of the DAI link$ref: /schemas/types.yaml#/definitions/stringdai-format:description: Specifies the serial data format of CPU DAI$ref: /schemas/types.yaml#/definitions/stringtesla,bitclock-master:description: Specifies the phandle of bitclock master DAI$ref: /schemas/types.yaml#/definitions/phandletesla,frame-master:description: Specifies the phandle of frameclock master DAI$ref: /schemas/types.yaml#/definitions/phandleThese are common properties. Drop 'tesla'.
Okay
cpu:description: Holds the CPU DAI node and instancetype: objectadditionalProperties: falseOkay
properties:sound-dai:description: Specifies the phandle of CPU DAI node$ref: /schemas/types.yaml#/definitions/phandle-arrayrequired:- sound-daicodec:description: Holds the Codec DAI node and instancetype: objectadditionalProperties: falseOkay
properties:sound-dai:description: Specifies the phandle of Codec DAI node$ref: /schemas/types.yaml#/definitions/phandle-arrayAlready has a type. Need to define how many entries (maxItems: 1 ?).
Okay. I'll update in the upcoming patch set
required:- sound-dai- required:
- link-name- dai-format- tesla,bitclock-master- tesla,frame-master- cpu+dependencies:
- dai-tdm-slot-width: [ 'dai-tdm-slot-num' ]
This should be captured with tdm-slot.txt converted to schema.
Okay
+required:
- compatible
- model
- dai-link
+additionalProperties: false
+examples:
- |
- sound {
compatible = "tesla,fsd-sndcard";status = "disabled";Why have a disabled example? Other than your example won't pass your schema.
Thanks for noticing, I'll double check and change the example keeping the status as enabled
No, just drop. Start with example-schema instead.
Best regards, Krzysztof
-----Original Message----- From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] Sent: 22 October 2022 10:19 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com; 'Rob Herring' robh@kernel.org Cc: lgirdwood@gmail.com; broonie@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com; alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung- soc@vger.kernel.org Subject: Re: [PATCH 3/6] dt-bindings: sound: Add sound card bindings for Tesla FSD
On 21/10/2022 04:44, Padmanabhan Rajanbabu wrote:
On Fri, Oct 14, 2022 at 03:51:48PM +0530, Padmanabhan Rajanbabu wrote:
Add dt-binding reference document to configure the DAI link for Tesla FSD sound card driver.
The simple-card or graph-card bindings don't work for you?
Thank you for reviewing the patch.
The actual reason for having a custom sound card driver lies in the fact that the Samsung i2s cpu dai requires configuration of some Samsung specific properties like rfs, bfs, codec clock direction and root clock source. We do not have flexibility of configuring the same in simple card driver (sound/soc/generic/simple-card.c) or audio graph card driver (sound/soc/generic/audio-graph-card.c). The binding has been added to support the custom sound card driver.
I understand from your query that the newly added card has device tree nodes and properties which are used in simple card as well, but having a different or new prefixes. I believe to address that, we can restructure the string names to generic ones.
You must use generic, existing properties where applicable.
Okay
But I would like to understand, to reuse the simple card or audio graph card bindings, can we add secondary compatible strings in the simple card dt-binding for the custom sound card to probe ?
If you see other vendor compatibles there, then yes... But there aren't any, right?
Yes you are right, we don't see other vendor compatibles. But, am I allowed to add such secondary compatibles so that we can extend the simple card and its utilities to a large extent?
If no, then I believe we will need a separate binding to extend the generic properties.
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
.../bindings/sound/tesla,fsd-card.yaml | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml
diff --git a/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml new file mode 100644 index 000000000000..4bd590f4ee27 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/tesla,fsd-card.yaml @@ -0,0 +1,158 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # +Copyright +2022 Samsung Electronics Co. Ltd. +%YAML 1.2 +--- +$id: +https://protect2.fireeye.com/v1/url?k=4ae54403-157e7d1c-4ae4cf4c-
000b
+abdfecba-9eb398ea304f8ae8&q=1&e=4935bed0-ce62-47dd-8faf-
4750b01e22d3&
+u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fsound%2Ftesla%2Cfsd-
card.ya
+ml%23 +$schema: +https://protect2.fireeye.com/v1/url?k=8c72226e-d3e91b71-8c73a921-
000b
+abdfecba-3ce999f6c052255b&q=1&e=4935bed0-ce62-47dd-8faf-
4750b01e22d3&
+u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
+title: Tesla FSD ASoC sound card driver
+maintainers:
- Padmanabhan Rajanbabu p.rajanbabu@samsung.com
+description: |
- This binding describes the node properties and essential DT
+entries of FSD
- SoC sound card node
+select: false
Never apply this schema. Why?
Sorry about it, let me change the select property to true in the next patchset
No, just drop it. Look at other bindings or at example-schema
+properties:
- compatible:
- enum:
- tesla,fsd-sndcard- model:
- description: Describes the Name of the sound card
- $ref: /schemas/types.yaml#/definitions/string
- widgets:
- description: A list of DAPM widgets in the sound card. Each
- entry
is a pair
of strings, the first being the widget name and the second- being
the
widget alias- $ref: /schemas/types.yaml#/definitions/string-array
- audio-routing:
- description: A list of the connections between audio components.
Each entry
is a pair of strings, the first being the connection's sink,- the
second
being the connection's source- $ref: /schemas/types.yaml#/definitions/string-array
- dai-tdm-slot-num:
- description: Enables TDM mode and specifies the number of TDM
- slots
to be
enabled- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [0, 1, 2, 3, 4, 5, 6, 7, 8]
maximum: 8
Okay
- default: 2
- dai-tdm-slot-width:
- description: Specifies the slot width of each TDm slot enabled
- $ref: /schemas/types.yaml#/definitions/uint32
- enum: [8, 16, 24]
- default: 16
All the above have types defined. You should not be redefining the types.
Okay
- dai-link:
- description: Holds the DAI link data between CPU, Codec and
Platform
- type: object
additionalProperties: falseokay
- properties:
link-name:description: Specifies the name of the DAI link$ref: /schemas/types.yaml#/definitions/stringdai-format:description: Specifies the serial data format of CPU DAI$ref: /schemas/types.yaml#/definitions/stringtesla,bitclock-master:description: Specifies the phandle of bitclock master DAI$ref: /schemas/types.yaml#/definitions/phandletesla,frame-master:description: Specifies the phandle of frameclock master DAI$ref: /schemas/types.yaml#/definitions/phandleThese are common properties. Drop 'tesla'.
Okay
cpu:description: Holds the CPU DAI node and instancetype: objectadditionalProperties: falseOkay
properties:sound-dai:description: Specifies the phandle of CPU DAI node$ref: /schemas/types.yaml#/definitions/phandle-arrayrequired:- sound-daicodec:description: Holds the Codec DAI node and instancetype: objectadditionalProperties: falseOkay
properties:sound-dai:description: Specifies the phandle of Codec DAI node$ref: /schemas/types.yaml#/definitions/phandle-arrayAlready has a type. Need to define how many entries (maxItems: 1 ?).
Okay. I'll update in the upcoming patch set
required:- sound-dai- required:
- link-name- dai-format- tesla,bitclock-master- tesla,frame-master- cpu+dependencies:
- dai-tdm-slot-width: [ 'dai-tdm-slot-num' ]
This should be captured with tdm-slot.txt converted to schema.
Okay
+required:
- compatible
- model
- dai-link
+additionalProperties: false
+examples:
- |
- sound {
compatible = "tesla,fsd-sndcard";status = "disabled";Why have a disabled example? Other than your example won't pass your schema.
Thanks for noticing, I'll double check and change the example keeping the status as enabled
No, just drop. Start with example-schema instead.
Best regards, Krzysztof
On 08/11/2022 06:33, Padmanabhan Rajanbabu wrote:
The actual reason for having a custom sound card driver lies in the fact that the Samsung i2s cpu dai requires configuration of some Samsung specific properties like rfs, bfs, codec clock direction and root clock source. We do not have flexibility of configuring the same in simple card driver (sound/soc/generic/simple-card.c) or audio graph card driver (sound/soc/generic/audio-graph-card.c). The binding has been added to support the custom sound card driver.
I understand from your query that the newly added card has device tree nodes and properties which are used in simple card as well, but having a different or new prefixes. I believe to address that, we can restructure the string names to generic ones.
You must use generic, existing properties where applicable.
Okay
But I would like to understand, to reuse the simple card or audio graph card bindings, can we add secondary compatible strings in the simple card dt-binding for the custom sound card to probe ?
If you see other vendor compatibles there, then yes... But there aren't any, right?
Yes you are right, we don't see other vendor compatibles. But, am I allowed to add such secondary compatibles so that we can extend the simple card and its utilities to a large extent?
If no, then I believe we will need a separate binding to extend the generic properties.
If you have any custom properties, then yes. But you don't have.
Best regards, Krzysztof
Add a soundcard driver for FSD audio interface to bridge the CPU DAI of samsung I2S with the codec and platform driver.
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com --- sound/soc/samsung/Kconfig | 12 ++ sound/soc/samsung/Makefile | 2 + sound/soc/samsung/fsd-card.c | 349 +++++++++++++++++++++++++++++++++++ 3 files changed, 363 insertions(+) create mode 100644 sound/soc/samsung/fsd-card.c
diff --git a/sound/soc/samsung/Kconfig b/sound/soc/samsung/Kconfig index 2a61e620cd3b..344503522bd3 100644 --- a/sound/soc/samsung/Kconfig +++ b/sound/soc/samsung/Kconfig @@ -239,4 +239,16 @@ config SND_SOC_SAMSUNG_MIDAS_WM1811 help Say Y if you want to add support for SoC audio on the Midas boards.
+config SND_SOC_TESLA_FSD + tristate "SoC I2S Audio support for Tesla FSD board" + depends on SND_SOC_SAMSUNG + select SND_SIMPLE_CARD_UTILS + select SND_SAMSUNG_I2S + help + Say Y if you want to add support for SOC audio on the FSD board. + This will select the SND_SIMPLE_CARD_UTILS to use the dummy + codec driver, incase the codec node is not specified in the device + tree. This sound card driver uses Samsung I2S controller as CPU + and platform DAI. + endif #SND_SOC_SAMSUNG diff --git a/sound/soc/samsung/Makefile b/sound/soc/samsung/Makefile index 398e843f388c..0dad88fdb1a8 100644 --- a/sound/soc/samsung/Makefile +++ b/sound/soc/samsung/Makefile @@ -43,6 +43,7 @@ snd-soc-arndale-objs := arndale.o snd-soc-tm2-wm5110-objs := tm2_wm5110.o snd-soc-aries-wm8994-objs := aries_wm8994.o snd-soc-midas-wm1811-objs := midas_wm1811.o +snd-soc-fsd-objs := fsd-card.o
obj-$(CONFIG_SND_SOC_SAMSUNG_JIVE_WM8750) += snd-soc-jive-wm8750.o obj-$(CONFIG_SND_SOC_SAMSUNG_NEO1973_WM8753) += snd-soc-neo1973-wm8753.o @@ -68,3 +69,4 @@ obj-$(CONFIG_SND_SOC_ARNDALE) += snd-soc-arndale.o obj-$(CONFIG_SND_SOC_SAMSUNG_TM2_WM5110) += snd-soc-tm2-wm5110.o obj-$(CONFIG_SND_SOC_SAMSUNG_ARIES_WM8994) += snd-soc-aries-wm8994.o obj-$(CONFIG_SND_SOC_SAMSUNG_MIDAS_WM1811) += snd-soc-midas-wm1811.o +obj-$(CONFIG_SND_SOC_TESLA_FSD) += snd-soc-fsd.o diff --git a/sound/soc/samsung/fsd-card.c b/sound/soc/samsung/fsd-card.c new file mode 100644 index 000000000000..806a4d3b99fd --- /dev/null +++ b/sound/soc/samsung/fsd-card.c @@ -0,0 +1,349 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * ALSA SoC Audio Layer - FSD Soundcard driver + * + * Copyright (c) 2022 Samsung Electronics Co. Ltd. + * Padmanabhan Rajanbabu p.rajanbabu@samsung.com + */ + + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/device.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> + +#include <sound/initval.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/soc-dai.h> +#include <sound/simple_card_utils.h> + +#include "i2s.h" +#include "i2s-regs.h" + +#define FSD_PREFIX "tesla," +#define FSD_DAI_SRC_PCLK 3 +#define FSD_DAI_RFS_192 192 +#define FSD_DAI_BFS_48 48 +#define FSD_DAI_BFS_96 96 +#define FSD_DAI_BFS_192 192 + +struct fsd_card_priv { + struct snd_soc_card card; + struct snd_soc_dai_link *dai_link; + u32 tdm_slots; + u32 tdm_slot_width; +}; + +static unsigned int fsd_card_get_rfs(unsigned int rate) +{ + return FSD_DAI_RFS_192; +} + +static unsigned int fsd_card_get_bfs(unsigned int channels) +{ + switch (channels) { + case 1: + case 2: + return FSD_DAI_BFS_48; + case 3: + case 4: + return FSD_DAI_BFS_96; + case 5: + case 6: + case 7: + case 8: + return FSD_DAI_BFS_192; + default: + return FSD_DAI_BFS_48; + } +} + +static unsigned int fsd_card_get_psr(unsigned int rate) +{ + switch (rate) { + case 8000: return 43; + case 11025: return 31; + case 16000: return 21; + case 22050: return 16; + case 32000: return 11; + case 44100: return 8; + case 48000: return 7; + case 64000: return 5; + case 88200: return 4; + case 96000: return 4; + case 192000: return 2; + default: return 1; + } +} + +static int fsd_card_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_card *card = rtd->card; + struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0); + struct snd_soc_dai_link *link = rtd->dai_link; + struct fsd_card_priv *priv = snd_soc_card_get_drvdata(card); + unsigned int rfs, bfs, psr; + int ret = 0, cdclk_dir; + + rfs = fsd_card_get_rfs(params_rate(params)); + bfs = fsd_card_get_bfs(params_channels(params)); + psr = fsd_card_get_psr(params_rate(params)); + + /* Configure the Root Clock Source */ + ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_OPCLK, + false, FSD_DAI_SRC_PCLK); + if (ret < 0) { + dev_err(card->dev, "Failed to set OPCLK: %d\n", ret); + goto err; + } + + ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_RCLKSRC_0, + false, false); + if (ret < 0) { + dev_err(card->dev, "Failed to set RCLKSRC: %d\n", ret); + goto err; + } + + /* Set CPU DAI configuration */ + ret = snd_soc_dai_set_fmt(cpu_dai, link->dai_fmt); + if (ret < 0) { + dev_err(card->dev, "Failed to set DAIFMT: %d\n", ret); + goto err; + } + + if (link->dai_fmt & SND_SOC_DAIFMT_CBC_CFC) { + cdclk_dir = SND_SOC_CLOCK_OUT; + } else if (link->dai_fmt & SND_SOC_DAIFMT_CBP_CFP) { + cdclk_dir = SND_SOC_CLOCK_IN; + } else { + dev_err(card->dev, "Missing Clock Master information\n"); + goto err; + } + + /* Set Clock Source for CDCLK */ + ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_CDCLK, + rfs, cdclk_dir); + if (ret < 0) { + dev_err(card->dev, "Failed to set CDCLK: %d\n", ret); + goto err; + } + + ret = snd_soc_dai_set_clkdiv(cpu_dai, SAMSUNG_I2S_DIV_RCLK, psr); + if (ret < 0) { + dev_err(card->dev, "Failed to set PSR: %d\n", ret); + goto err; + } + + ret = snd_soc_dai_set_clkdiv(cpu_dai, SAMSUNG_I2S_DIV_BCLK, bfs); + if (ret < 0) { + dev_err(card->dev, "Failed to set BCLK: %d\n", ret); + goto err; + } + + if (priv->tdm_slots) { + ret = snd_soc_dai_set_tdm_slot(cpu_dai, false, false, + priv->tdm_slots, priv->tdm_slot_width); + if (ret < 0) { + dev_err(card->dev, + "Failed to configure in TDM mode:%d\n", ret); + goto err; + } + } + +err: + return ret; +} + +static const struct snd_soc_ops fsd_card_ops = { + .hw_params = fsd_card_hw_params, +}; + +static struct fsd_card_priv *fsd_card_parse_of(struct snd_soc_card *card) +{ + struct fsd_card_priv *priv; + struct snd_soc_dai_link *link; + struct device *dev = card->dev; + struct device_node *node = dev->of_node; + struct device_node *np, *cpu_node, *codec_node; + struct snd_soc_dai_link_component *dlc; + int ret, id = 0, num_links; + + ret = snd_soc_of_parse_card_name(card, "model"); + if (ret) { + dev_err(dev, "Error parsing card name: %d\n", ret); + return ERR_PTR(ret); + } + + if (of_property_read_bool(dev->of_node, "widgets")) { + ret = snd_soc_of_parse_audio_simple_widgets(card, "widgets"); + if (ret) + return ERR_PTR(ret); + } + + /* Add DAPM routes to the card */ + if (of_property_read_bool(node, "audio-routing")) { + ret = snd_soc_of_parse_audio_routing(card, "audio-routing"); + if (ret) + return ERR_PTR(ret); + } + + num_links = of_get_child_count(node); + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return ERR_PTR(-ENOMEM); + + priv->dai_link = devm_kzalloc(dev, num_links * sizeof(*priv->dai_link), + GFP_KERNEL); + if (!priv->dai_link) + return ERR_PTR(-ENOMEM); + + priv->tdm_slots = 0; + priv->tdm_slot_width = 0; + + snd_soc_of_parse_tdm_slot(node, NULL, NULL, &priv->tdm_slots, + &priv->tdm_slot_width); + + link = priv->dai_link; + + for_each_child_of_node(node, np) { + dlc = devm_kzalloc(dev, 2 * sizeof(*dlc), GFP_KERNEL); + if (!dlc) + return ERR_PTR(-ENOMEM); + + link->id = id; + link->cpus = &dlc[0]; + link->platforms = &dlc[1]; + link->num_cpus = 1; + link->num_codecs = 1; + link->num_platforms = 1; + + cpu_node = of_get_child_by_name(np, "cpu"); + if (!cpu_node) { + dev_err(dev, "Missing CPU/Codec node\n"); + ret = -EINVAL; + goto err_cpu_node; + } + + ret = snd_soc_of_get_dai_link_cpus(dev, cpu_node, link); + if (ret < 0) { + dev_err(dev, "Error Parsing CPU DAI name\n"); + goto err_cpu_name; + } + + link->platforms->of_node = link->cpus->of_node; + + codec_node = of_get_child_by_name(np, "codec"); + if (codec_node) { + ret = snd_soc_of_get_dai_link_codecs(dev, codec_node, + link); + if (ret < 0) { + dev_err(dev, "Error Parsing Codec DAI name\n"); + goto err_codec_name; + } + } else { + dlc = devm_kzalloc(dev, sizeof(*dlc), GFP_KERNEL); + if (!dlc) { + ret = -ENOMEM; + goto err_cpu_name; + } + + link->codecs = dlc; + + link->codecs->dai_name = "snd-soc-dummy-dai"; + link->codecs->name = "snd-soc-dummy"; + link->dynamic = 1; + + snd_soc_dai_link_set_capabilities(link); + link->ignore_suspend = 1; + link->nonatomic = 1; + } + + ret = asoc_simple_parse_daifmt(dev, np, codec_node, + FSD_PREFIX, &link->dai_fmt); + if (ret) + link->dai_fmt = SND_SOC_DAIFMT_NB_NF | + SND_SOC_DAIFMT_CBC_CFC | + SND_SOC_DAIFMT_I2S; + + ret = of_property_read_string(np, "link-name", &link->name); + if (ret) { + dev_err(card->dev, "Error Parsing link name\n"); + goto err_codec_name; + } + + link->stream_name = link->name; + link->ops = &fsd_card_ops; + + link++; + id++; + + of_node_put(cpu_node); + of_node_put(codec_node); + } + + card->dai_link = priv->dai_link; + card->num_links = num_links; + + return priv; + +err_codec_name: + of_node_put(codec_node); +err_cpu_name: + of_node_put(cpu_node); +err_cpu_node: + of_node_put(np); + return ERR_PTR(ret); +} + +static int fsd_platform_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct snd_soc_card *card; + struct fsd_card_priv *fsd_priv; + + card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL); + if (!card) + return -ENOMEM; + + card->dev = dev; + fsd_priv = fsd_card_parse_of(card); + + if (IS_ERR(fsd_priv)) { + dev_err(&pdev->dev, "Error Parsing DAI Link: %ld\n", + PTR_ERR(fsd_priv)); + return PTR_ERR(fsd_priv); + } + + snd_soc_card_set_drvdata(card, fsd_priv); + + return devm_snd_soc_register_card(&pdev->dev, card); +} + +static const struct of_device_id fsd_device_id[] = { + { .compatible = "tesla,fsd-card" }, + {}, +}; +MODULE_DEVICE_TABLE(of, fsd_device_id); + +static struct platform_driver fsd_platform_driver = { + .driver = { + .name = "fsd-card", + .of_match_table = of_match_ptr(fsd_device_id), + }, + .probe = fsd_platform_probe, +}; +module_platform_driver(fsd_platform_driver); + +MODULE_AUTHOR("Padmanabhan Rajanbabu p.rajanbabu@samsung.com"); +MODULE_DESCRIPTION("FSD ASoC Soundcard Driver"); +MODULE_LICENSE("GPL");
On Fri, Oct 14, 2022 at 03:51:49PM +0530, Padmanabhan Rajanbabu wrote:
+++ b/sound/soc/samsung/fsd-card.c @@ -0,0 +1,349 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- ALSA SoC Audio Layer - FSD Soundcard driver
- Copyright (c) 2022 Samsung Electronics Co. Ltd.
- Padmanabhan Rajanbabu p.rajanbabu@samsung.com
Please make the entire comment a C++ one so things look more intentional.
- if (link->dai_fmt & SND_SOC_DAIFMT_CBC_CFC) {
cdclk_dir = SND_SOC_CLOCK_OUT;- } else if (link->dai_fmt & SND_SOC_DAIFMT_CBP_CFP) {
cdclk_dir = SND_SOC_CLOCK_IN;- } else {
dev_err(card->dev, "Missing Clock Master information\n");goto err;- }
We're trying to modernise the langauge around clock providers, please use that term rather than the outdated terminology here.
- if (priv->tdm_slots) {
ret = snd_soc_dai_set_tdm_slot(cpu_dai, false, false,priv->tdm_slots, priv->tdm_slot_width);if (ret < 0) {dev_err(card->dev,"Failed to configure in TDM mode:%d\n", ret);goto err;}- }
Just set things once on probe if they don't depend on the configuration, it's neater and marginally faster if nothing else.
- if (of_property_read_bool(dev->of_node, "widgets")) {
ret = snd_soc_of_parse_audio_simple_widgets(card, "widgets");if (ret)return ERR_PTR(ret);- }
- /* Add DAPM routes to the card */
- if (of_property_read_bool(node, "audio-routing")) {
ret = snd_soc_of_parse_audio_routing(card, "audio-routing");if (ret)return ERR_PTR(ret);- }
Just fix the library functions to handle missing properties gracefully, every card is going to need the same code here.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: 14 October 2022 05:53 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com Cc: lgirdwood@gmail.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com; alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung- soc@vger.kernel.org Subject: Re: [PATCH 4/6] ASoC: samsung: fsd: Add FSD soundcard driver
On Fri, Oct 14, 2022 at 03:51:49PM +0530, Padmanabhan Rajanbabu wrote:
+++ b/sound/soc/samsung/fsd-card.c @@ -0,0 +1,349 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- ALSA SoC Audio Layer - FSD Soundcard driver
- Copyright (c) 2022 Samsung Electronics Co. Ltd.
- Padmanabhan Rajanbabu p.rajanbabu@samsung.com
Please make the entire comment a C++ one so things look more intentional.
okay
- if (link->dai_fmt & SND_SOC_DAIFMT_CBC_CFC) {
cdclk_dir = SND_SOC_CLOCK_OUT;- } else if (link->dai_fmt & SND_SOC_DAIFMT_CBP_CFP) {
cdclk_dir = SND_SOC_CLOCK_IN;- } else {
dev_err(card->dev, "Missing Clock Master information\n");goto err;- }
We're trying to modernise the langauge around clock providers, please use that term rather than the outdated terminology here.
Okay, I'll make the necessary change for the clock terminologies in the next patch set
- if (priv->tdm_slots) {
ret = snd_soc_dai_set_tdm_slot(cpu_dai, false, false,priv->tdm_slots, priv->tdm_slot_width);if (ret < 0) {dev_err(card->dev,"Failed to configure in TDM mode:%d\n",ret);
goto err;}- }
Just set things once on probe if they don't depend on the configuration,
it's
neater and marginally faster if nothing else.
Okay
- if (of_property_read_bool(dev->of_node, "widgets")) {
ret = snd_soc_of_parse_audio_simple_widgets(card,"widgets");
if (ret)return ERR_PTR(ret);- }
- /* Add DAPM routes to the card */
- if (of_property_read_bool(node, "audio-routing")) {
ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
if (ret)return ERR_PTR(ret);- }
Just fix the library functions to handle missing properties gracefully,
every
card is going to need the same code here.
Okay
Thank you for reviewing the patch
On 14/10/2022 06:21, Padmanabhan Rajanbabu wrote:
Add a soundcard driver for FSD audio interface to bridge the CPU DAI of samsung I2S with the codec and platform driver.
Thank you for your patch. There is something to discuss/improve.
+#define FSD_PREFIX "tesla," +#define FSD_DAI_SRC_PCLK 3 +#define FSD_DAI_RFS_192 192 +#define FSD_DAI_BFS_48 48 +#define FSD_DAI_BFS_96 96 +#define FSD_DAI_BFS_192 192
+struct fsd_card_priv {
- struct snd_soc_card card;
- struct snd_soc_dai_link *dai_link;
- u32 tdm_slots;
- u32 tdm_slot_width;
+};
+static unsigned int fsd_card_get_rfs(unsigned int rate) +{
- return FSD_DAI_RFS_192;
This wrapper is a bit silly...
+}
+static unsigned int fsd_card_get_bfs(unsigned int channels) +{
- switch (channels) {
- case 1:
- case 2:
return FSD_DAI_BFS_48;- case 3:
- case 4:
return FSD_DAI_BFS_96;- case 5:
- case 6:
- case 7:
- case 8:
return FSD_DAI_BFS_192;- default:
return FSD_DAI_BFS_48;- }
+}
+static unsigned int fsd_card_get_psr(unsigned int rate) +{
- switch (rate) {
- case 8000: return 43;
- case 11025: return 31;
- case 16000: return 21;
- case 22050: return 16;
- case 32000: return 11;
- case 44100: return 8;
- case 48000: return 7;
- case 64000: return 5;
- case 88200: return 4;
- case 96000: return 4;
- case 192000: return 2;
- default: return 1;
- }
+}
+static int fsd_card_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params)+{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_card *card = rtd->card;
- struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
- struct snd_soc_dai_link *link = rtd->dai_link;
- struct fsd_card_priv *priv = snd_soc_card_get_drvdata(card);
- unsigned int rfs, bfs, psr;
- int ret = 0, cdclk_dir;
- rfs = fsd_card_get_rfs(params_rate(params));
- bfs = fsd_card_get_bfs(params_channels(params));
- psr = fsd_card_get_psr(params_rate(params));
- /* Configure the Root Clock Source */
- ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_OPCLK,
false, FSD_DAI_SRC_PCLK);- if (ret < 0) {
dev_err(card->dev, "Failed to set OPCLK: %d\n", ret);goto err;- }
- ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_RCLKSRC_0,
false, false);- if (ret < 0) {
dev_err(card->dev, "Failed to set RCLKSRC: %d\n", ret);
Don't you need to cleanup on error paths?
goto err;- }
- /* Set CPU DAI configuration */
- ret = snd_soc_dai_set_fmt(cpu_dai, link->dai_fmt);
- if (ret < 0) {
dev_err(card->dev, "Failed to set DAIFMT: %d\n", ret);goto err;- }
- if (link->dai_fmt & SND_SOC_DAIFMT_CBC_CFC) {
cdclk_dir = SND_SOC_CLOCK_OUT;- } else if (link->dai_fmt & SND_SOC_DAIFMT_CBP_CFP) {
cdclk_dir = SND_SOC_CLOCK_IN;- } else {
dev_err(card->dev, "Missing Clock Master information\n");goto err;- }
- /* Set Clock Source for CDCLK */
- ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_CDCLK,
rfs, cdclk_dir);- if (ret < 0) {
dev_err(card->dev, "Failed to set CDCLK: %d\n", ret);goto err;- }
- ret = snd_soc_dai_set_clkdiv(cpu_dai, SAMSUNG_I2S_DIV_RCLK, psr);
- if (ret < 0) {
dev_err(card->dev, "Failed to set PSR: %d\n", ret);goto err;- }
- ret = snd_soc_dai_set_clkdiv(cpu_dai, SAMSUNG_I2S_DIV_BCLK, bfs);
- if (ret < 0) {
dev_err(card->dev, "Failed to set BCLK: %d\n", ret);goto err;- }
- if (priv->tdm_slots) {
ret = snd_soc_dai_set_tdm_slot(cpu_dai, false, false,priv->tdm_slots, priv->tdm_slot_width);if (ret < 0) {dev_err(card->dev,"Failed to configure in TDM mode:%d\n", ret);goto err;}- }
+err:
- return ret;
+}
+static const struct snd_soc_ops fsd_card_ops = {
- .hw_params = fsd_card_hw_params,
+};
+static struct fsd_card_priv *fsd_card_parse_of(struct snd_soc_card *card) +{
- struct fsd_card_priv *priv;
- struct snd_soc_dai_link *link;
- struct device *dev = card->dev;
- struct device_node *node = dev->of_node;
- struct device_node *np, *cpu_node, *codec_node;
- struct snd_soc_dai_link_component *dlc;
- int ret, id = 0, num_links;
- ret = snd_soc_of_parse_card_name(card, "model");
- if (ret) {
dev_err(dev, "Error parsing card name: %d\n", ret);return ERR_PTR(ret);
return dev_err_probe
- }
- if (of_property_read_bool(dev->of_node, "widgets")) {
ret = snd_soc_of_parse_audio_simple_widgets(card, "widgets");if (ret)return ERR_PTR(ret);- }
- /* Add DAPM routes to the card */
- if (of_property_read_bool(node, "audio-routing")) {
ret = snd_soc_of_parse_audio_routing(card, "audio-routing");if (ret)return ERR_PTR(ret);- }
- num_links = of_get_child_count(node);
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
return ERR_PTR(-ENOMEM);- priv->dai_link = devm_kzalloc(dev, num_links * sizeof(*priv->dai_link),
GFP_KERNEL);- if (!priv->dai_link)
return ERR_PTR(-ENOMEM);- priv->tdm_slots = 0;
- priv->tdm_slot_width = 0;
- snd_soc_of_parse_tdm_slot(node, NULL, NULL, &priv->tdm_slots,
&priv->tdm_slot_width);- link = priv->dai_link;
- for_each_child_of_node(node, np) {
dlc = devm_kzalloc(dev, 2 * sizeof(*dlc), GFP_KERNEL);if (!dlc)return ERR_PTR(-ENOMEM);link->id = id;link->cpus = &dlc[0];link->platforms = &dlc[1];link->num_cpus = 1;link->num_codecs = 1;link->num_platforms = 1;cpu_node = of_get_child_by_name(np, "cpu");if (!cpu_node) {dev_err(dev, "Missing CPU/Codec node\n");ret = -EINVAL;goto err_cpu_node;}ret = snd_soc_of_get_dai_link_cpus(dev, cpu_node, link);if (ret < 0) {dev_err(dev, "Error Parsing CPU DAI name\n");goto err_cpu_name;}link->platforms->of_node = link->cpus->of_node;codec_node = of_get_child_by_name(np, "codec");if (codec_node) {ret = snd_soc_of_get_dai_link_codecs(dev, codec_node,link);if (ret < 0) {dev_err(dev, "Error Parsing Codec DAI name\n");goto err_codec_name;}} else {dlc = devm_kzalloc(dev, sizeof(*dlc), GFP_KERNEL);if (!dlc) {ret = -ENOMEM;goto err_cpu_name;}link->codecs = dlc;link->codecs->dai_name = "snd-soc-dummy-dai";link->codecs->name = "snd-soc-dummy";link->dynamic = 1;snd_soc_dai_link_set_capabilities(link);link->ignore_suspend = 1;link->nonatomic = 1;}ret = asoc_simple_parse_daifmt(dev, np, codec_node,FSD_PREFIX, &link->dai_fmt);if (ret)link->dai_fmt = SND_SOC_DAIFMT_NB_NF |SND_SOC_DAIFMT_CBC_CFC |SND_SOC_DAIFMT_I2S;ret = of_property_read_string(np, "link-name", &link->name);if (ret) {dev_err(card->dev, "Error Parsing link name\n");goto err_codec_name;}link->stream_name = link->name;link->ops = &fsd_card_ops;link++;id++;of_node_put(cpu_node);of_node_put(codec_node);- }
- card->dai_link = priv->dai_link;
- card->num_links = num_links;
- return priv;
+err_codec_name:
- of_node_put(codec_node);
+err_cpu_name:
- of_node_put(cpu_node);
+err_cpu_node:
- of_node_put(np);
- return ERR_PTR(ret);
+}
+static int fsd_platform_probe(struct platform_device *pdev) +{
- struct device *dev = &pdev->dev;
- struct snd_soc_card *card;
- struct fsd_card_priv *fsd_priv;
- card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL);
- if (!card)
return -ENOMEM;- card->dev = dev;
- fsd_priv = fsd_card_parse_of(card);
Drop the indentation before =
- if (IS_ERR(fsd_priv)) {
dev_err(&pdev->dev, "Error Parsing DAI Link: %ld\n",PTR_ERR(fsd_priv));return PTR_ERR(fsd_priv);
return dev_err_probe
- }
- snd_soc_card_set_drvdata(card, fsd_priv);
- return devm_snd_soc_register_card(&pdev->dev, card);
+}
+static const struct of_device_id fsd_device_id[] = {
- { .compatible = "tesla,fsd-card" },
- {},
+}; +MODULE_DEVICE_TABLE(of, fsd_device_id);
+static struct platform_driver fsd_platform_driver = {
- .driver = {
.name = "fsd-card",.of_match_table = of_match_ptr(fsd_device_id),
of_match_ptr comes with maybe_unused. Or drop it.
Best regards, Krzysztof
-----Original Message----- From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] Sent: 16 October 2022 08:48 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com; lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 4/6] ASoC: samsung: fsd: Add FSD soundcard driver
On 14/10/2022 06:21, Padmanabhan Rajanbabu wrote:
Add a soundcard driver for FSD audio interface to bridge the CPU DAI of samsung I2S with the codec and platform driver.
Thank you for your patch. There is something to discuss/improve.
+#define FSD_PREFIX "tesla," +#define FSD_DAI_SRC_PCLK 3 +#define FSD_DAI_RFS_192 192 +#define FSD_DAI_BFS_48 48 +#define FSD_DAI_BFS_96 96 +#define FSD_DAI_BFS_192 192
+struct fsd_card_priv {
- struct snd_soc_card card;
- struct snd_soc_dai_link *dai_link;
- u32 tdm_slots;
- u32 tdm_slot_width;
+};
+static unsigned int fsd_card_get_rfs(unsigned int rate) {
- return FSD_DAI_RFS_192;
This wrapper is a bit silly...
okay, will remove the wrapper and assign the macro value directly
+}
+static unsigned int fsd_card_get_bfs(unsigned int channels) {
- switch (channels) {
- case 1:
- case 2:
return FSD_DAI_BFS_48;- case 3:
- case 4:
return FSD_DAI_BFS_96;- case 5:
- case 6:
- case 7:
- case 8:
return FSD_DAI_BFS_192;- default:
return FSD_DAI_BFS_48;- }
+}
+static unsigned int fsd_card_get_psr(unsigned int rate) {
- switch (rate) {
- case 8000: return 43;
- case 11025: return 31;
- case 16000: return 21;
- case 22050: return 16;
- case 32000: return 11;
- case 44100: return 8;
- case 48000: return 7;
- case 64000: return 5;
- case 88200: return 4;
- case 96000: return 4;
- case 192000: return 2;
- default: return 1;
- }
+}
+static int fsd_card_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params*params) {
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_card *card = rtd->card;
- struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
- struct snd_soc_dai_link *link = rtd->dai_link;
- struct fsd_card_priv *priv = snd_soc_card_get_drvdata(card);
- unsigned int rfs, bfs, psr;
- int ret = 0, cdclk_dir;
- rfs = fsd_card_get_rfs(params_rate(params));
- bfs = fsd_card_get_bfs(params_channels(params));
- psr = fsd_card_get_psr(params_rate(params));
- /* Configure the Root Clock Source */
- ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_OPCLK,
false, FSD_DAI_SRC_PCLK);- if (ret < 0) {
dev_err(card->dev, "Failed to set OPCLK: %d\n", ret);goto err;- }
- ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_RCLKSRC_0,
false, false);- if (ret < 0) {
dev_err(card->dev, "Failed to set RCLKSRC: %d\n", ret);Don't you need to cleanup on error paths?
we might not be needing, since the sound card neither configures any clock sources directly nor involves in any memory allocation during hw_params.
goto err;- }
- /* Set CPU DAI configuration */
- ret = snd_soc_dai_set_fmt(cpu_dai, link->dai_fmt);
- if (ret < 0) {
dev_err(card->dev, "Failed to set DAIFMT: %d\n", ret);goto err;- }
- if (link->dai_fmt & SND_SOC_DAIFMT_CBC_CFC) {
cdclk_dir = SND_SOC_CLOCK_OUT;- } else if (link->dai_fmt & SND_SOC_DAIFMT_CBP_CFP) {
cdclk_dir = SND_SOC_CLOCK_IN;- } else {
dev_err(card->dev, "Missing Clock Master information\n");goto err;- }
- /* Set Clock Source for CDCLK */
- ret = snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_CDCLK,
rfs, cdclk_dir);- if (ret < 0) {
dev_err(card->dev, "Failed to set CDCLK: %d\n", ret);goto err;- }
- ret = snd_soc_dai_set_clkdiv(cpu_dai, SAMSUNG_I2S_DIV_RCLK, psr);
- if (ret < 0) {
dev_err(card->dev, "Failed to set PSR: %d\n", ret);goto err;- }
- ret = snd_soc_dai_set_clkdiv(cpu_dai, SAMSUNG_I2S_DIV_BCLK, bfs);
- if (ret < 0) {
dev_err(card->dev, "Failed to set BCLK: %d\n", ret);goto err;- }
- if (priv->tdm_slots) {
ret = snd_soc_dai_set_tdm_slot(cpu_dai, false, false,priv->tdm_slots, priv->tdm_slot_width);if (ret < 0) {dev_err(card->dev,"Failed to configure in TDM mode:%d\n", ret);goto err;}- }
+err:
- return ret;
+}
+static const struct snd_soc_ops fsd_card_ops = {
- .hw_params = fsd_card_hw_params,
+};
+static struct fsd_card_priv *fsd_card_parse_of(struct snd_soc_card +*card) {
- struct fsd_card_priv *priv;
- struct snd_soc_dai_link *link;
- struct device *dev = card->dev;
- struct device_node *node = dev->of_node;
- struct device_node *np, *cpu_node, *codec_node;
- struct snd_soc_dai_link_component *dlc;
- int ret, id = 0, num_links;
- ret = snd_soc_of_parse_card_name(card, "model");
- if (ret) {
dev_err(dev, "Error parsing card name: %d\n", ret);return ERR_PTR(ret);return dev_err_probe
Okay
- }
- if (of_property_read_bool(dev->of_node, "widgets")) {
ret = snd_soc_of_parse_audio_simple_widgets(card, "widgets");if (ret)return ERR_PTR(ret);- }
- /* Add DAPM routes to the card */
- if (of_property_read_bool(node, "audio-routing")) {
ret = snd_soc_of_parse_audio_routing(card, "audio-routing");if (ret)return ERR_PTR(ret);- }
- num_links = of_get_child_count(node);
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
return ERR_PTR(-ENOMEM);- priv->dai_link = devm_kzalloc(dev, num_links * sizeof(*priv-
dai_link),
- GFP_KERNEL);
- if (!priv->dai_link)
return ERR_PTR(-ENOMEM);- priv->tdm_slots = 0;
- priv->tdm_slot_width = 0;
- snd_soc_of_parse_tdm_slot(node, NULL, NULL, &priv->tdm_slots,
&priv->tdm_slot_width);- link = priv->dai_link;
- for_each_child_of_node(node, np) {
dlc = devm_kzalloc(dev, 2 * sizeof(*dlc), GFP_KERNEL);if (!dlc)return ERR_PTR(-ENOMEM);link->id = id;link->cpus = &dlc[0];link->platforms = &dlc[1];link->num_cpus = 1;link->num_codecs = 1;link->num_platforms = 1;cpu_node = of_get_child_by_name(np, "cpu");if (!cpu_node) {dev_err(dev, "Missing CPU/Codec node\n");ret = -EINVAL;goto err_cpu_node;}ret = snd_soc_of_get_dai_link_cpus(dev, cpu_node, link);if (ret < 0) {dev_err(dev, "Error Parsing CPU DAI name\n");goto err_cpu_name;}link->platforms->of_node = link->cpus->of_node;codec_node = of_get_child_by_name(np, "codec");if (codec_node) {ret = snd_soc_of_get_dai_link_codecs(dev, codec_node,link);if (ret < 0) {dev_err(dev, "Error Parsing Codec DAI name\n");goto err_codec_name;}} else {dlc = devm_kzalloc(dev, sizeof(*dlc), GFP_KERNEL);if (!dlc) {ret = -ENOMEM;goto err_cpu_name;}link->codecs = dlc;link->codecs->dai_name = "snd-soc-dummy-dai";link->codecs->name = "snd-soc-dummy";link->dynamic = 1;snd_soc_dai_link_set_capabilities(link);link->ignore_suspend = 1;link->nonatomic = 1;}ret = asoc_simple_parse_daifmt(dev, np, codec_node,FSD_PREFIX, &link->dai_fmt);if (ret)link->dai_fmt = SND_SOC_DAIFMT_NB_NF |SND_SOC_DAIFMT_CBC_CFC |SND_SOC_DAIFMT_I2S;ret = of_property_read_string(np, "link-name", &link-name);
if (ret) {dev_err(card->dev, "Error Parsing link name\n");goto err_codec_name;}link->stream_name = link->name;link->ops = &fsd_card_ops;link++;id++;of_node_put(cpu_node);of_node_put(codec_node);- }
- card->dai_link = priv->dai_link;
- card->num_links = num_links;
- return priv;
+err_codec_name:
- of_node_put(codec_node);
+err_cpu_name:
- of_node_put(cpu_node);
+err_cpu_node:
- of_node_put(np);
- return ERR_PTR(ret);
+}
+static int fsd_platform_probe(struct platform_device *pdev) {
- struct device *dev = &pdev->dev;
- struct snd_soc_card *card;
- struct fsd_card_priv *fsd_priv;
- card = devm_kzalloc(dev, sizeof(*card), GFP_KERNEL);
- if (!card)
return -ENOMEM;- card->dev = dev;
- fsd_priv = fsd_card_parse_of(card);
Drop the indentation before =
Okay
- if (IS_ERR(fsd_priv)) {
dev_err(&pdev->dev, "Error Parsing DAI Link: %ld\n",PTR_ERR(fsd_priv));return PTR_ERR(fsd_priv);return dev_err_probe
Okay
- }
- snd_soc_card_set_drvdata(card, fsd_priv);
- return devm_snd_soc_register_card(&pdev->dev, card); }
+static const struct of_device_id fsd_device_id[] = {
- { .compatible = "tesla,fsd-card" },
- {},
+}; +MODULE_DEVICE_TABLE(of, fsd_device_id);
+static struct platform_driver fsd_platform_driver = {
- .driver = {
.name = "fsd-card",.of_match_table = of_match_ptr(fsd_device_id),of_match_ptr comes with maybe_unused. Or drop it.
Okay
Best regards, Krzysztof
Thank you for reviewing the patch.
Add device tree node for I2S0 and I2S1 CPU DAI instances for Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com --- arch/arm64/boot/dts/tesla/fsd-evb.dts | 8 +++++ arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 14 ++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 38 ++++++++++++++++++++++ 3 files changed, 60 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index 1db6ddf03f01..c0a4509499ab 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -41,3 +41,11 @@ &ufs { status = "okay"; }; + +&tdm_0 { + status = "okay"; +}; + +&tdm_1 { + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi index e3852c946352..ff6f5d4b16dd 100644 --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi @@ -339,6 +339,20 @@ samsung,pin-pud = <FSD_PIN_PULL_UP>; samsung,pin-drv = <FSD_PIN_DRV_LV4>; }; + + i2s0_bus: i2s0-bus { + samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2", "gpd1-3", "gpd1-4"; + samsung,pin-function = <FSD_PIN_FUNC_2>; + samsung,pin-pud = <FSD_PIN_PULL_DOWN>; + samsung,pin-drv = <FSD_PIN_DRV_LV4>; + }; + + i2s1_bus: i2s1-bus { + samsung,pins = "gpd2-0", "gpd2-1", "gpd2-2", "gpd2-3", "gpd2-4"; + samsung,pin-function = <FSD_PIN_FUNC_2>; + samsung,pin-pud = <FSD_PIN_PULL_DOWN>; + samsung,pin-drv = <FSD_PIN_DRV_LV4>; + }; };
&pinctrl_pmu { diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index f35bc5a288c2..5decad45a1b6 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -32,6 +32,8 @@ spi0 = &spi_0; spi1 = &spi_1; spi2 = &spi_2; + tdm0 = &tdm_0; + tdm1 = &tdm_1; };
cpus { @@ -809,6 +811,42 @@ status = "disabled"; };
+ tdm_0: tdm@140e0000 { + compatible = "samsung,exynos7-i2s"; + reg = <0x0 0x140E0000 0x0 0x100>; + interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>; + dmas = <&pdma1 14>, <&pdma1 13>, <&pdma1 12>; + dma-names = "tx", "rx", "tx-sec"; + #clock-cells = <1>; + #sound-dai-cells = <1>; + clocks = <&clock_peric PERIC_HCLK_TDM0>, + <&clock_peric PERIC_HCLK_TDM0>, + <&clock_peric PERIC_PCLK_TDM0>; + clock-names = "i2s_opclk0", "i2s_opclk1", "iis"; + pinctrl-names = "default"; + pinctrl-0 = <&i2s0_bus>; + samsung,sec-dai-id = <0>; + status = "disabled"; + }; + + tdm_1: tdm@140f0000 { + compatible = "samsung,exynos7-i2s"; + reg = <0x0 0x140F0000 0x0 0x100>; + interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>; + dmas = <&pdma1 17>, <&pdma1 16>, <&pdma1 15>; + dma-names = "tx", "rx", "tx-sec"; + #clock-cells = <1>; + #sound-dai-cells = <1>; + clocks = <&clock_peric PERIC_HCLK_TDM1>, + <&clock_peric PERIC_HCLK_TDM1>, + <&clock_peric PERIC_PCLK_TDM1>; + clock-names = "i2s_opclk0", "i2s_opclk1", "iis"; + pinctrl-names = "default"; + pinctrl-0 = <&i2s1_bus>; + samsung,sec-dai-id = <1>; + status = "disabled"; + }; + timer@10040000 { compatible = "tesla,fsd-mct", "samsung,exynos4210-mct"; reg = <0x0 0x10040000 0x0 0x800>;
-----Original Message----- From: Padmanabhan Rajanbabu [mailto:p.rajanbabu@samsung.com] Sent: Friday, October 14, 2022 3:52 PM To: lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org; Padmanabhan Rajanbabu p.rajanbabu@samsung.com Subject: [PATCH 5/6] arm64: dts: fsd: Add I2S DAI node for Tesla FSD
Add device tree node for I2S0 and I2S1 CPU DAI instances for Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
arch/arm64/boot/dts/tesla/fsd-evb.dts | 8 +++++ arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 14 ++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 38 ++++++++++++++++++++++ 3 files changed, 60 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index 1db6ddf03f01..c0a4509499ab 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -41,3 +41,11 @@ &ufs { status = "okay"; };
+&tdm_0 {
- status = "okay";
+};
+&tdm_1 {
- status = "okay";
+}; diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi index e3852c946352..ff6f5d4b16dd 100644 --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi @@ -339,6 +339,20 @@ samsung,pin-pud = <FSD_PIN_PULL_UP>; samsung,pin-drv = <FSD_PIN_DRV_LV4>; };
- i2s0_bus: i2s0-bus {
samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2", "gpd1-3","gpd1-4";
samsung,pin-function = <FSD_PIN_FUNC_2>;samsung,pin-pud = <FSD_PIN_PULL_DOWN>;samsung,pin-drv = <FSD_PIN_DRV_LV4>;- };
- i2s1_bus: i2s1-bus {
samsung,pins = "gpd2-0", "gpd2-1", "gpd2-2", "gpd2-3","gpd2-4";
samsung,pin-function = <FSD_PIN_FUNC_2>;samsung,pin-pud = <FSD_PIN_PULL_DOWN>;samsung,pin-drv = <FSD_PIN_DRV_LV4>;- };
};
&pinctrl_pmu { diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index f35bc5a288c2..5decad45a1b6 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -32,6 +32,8 @@ spi0 = &spi_0; spi1 = &spi_1; spi2 = &spi_2;
tdm0 = &tdm_0;tdm1 = &tdm_1;};
cpus {
@@ -809,6 +811,42 @@ status = "disabled"; };
tdm_0: tdm@140e0000 {compatible = "samsung,exynos7-i2s";reg = <0x0 0x140E0000 0x0 0x100>;
Address should be all in small caps Make sure you have run 'make dtbs_check'
interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;dmas = <&pdma1 14>, <&pdma1 13>, <&pdma1 12>;dma-names = "tx", "rx", "tx-sec";#clock-cells = <1>;#sound-dai-cells = <1>;clocks = <&clock_peric PERIC_HCLK_TDM0>,<&clock_peric PERIC_HCLK_TDM0>,<&clock_peric PERIC_PCLK_TDM0>;clock-names = "i2s_opclk0", "i2s_opclk1", "iis";pinctrl-names = "default";pinctrl-0 = <&i2s0_bus>;samsung,sec-dai-id = <0>;status = "disabled";};tdm_1: tdm@140f0000 {compatible = "samsung,exynos7-i2s";reg = <0x0 0x140F0000 0x0 0x100>;
Same as above
interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>;dmas = <&pdma1 17>, <&pdma1 16>, <&pdma1 15>;dma-names = "tx", "rx", "tx-sec";#clock-cells = <1>;#sound-dai-cells = <1>;clocks = <&clock_peric PERIC_HCLK_TDM1>,<&clock_peric PERIC_HCLK_TDM1>,<&clock_peric PERIC_PCLK_TDM1>;clock-names = "i2s_opclk0", "i2s_opclk1", "iis";pinctrl-names = "default";pinctrl-0 = <&i2s1_bus>;samsung,sec-dai-id = <1>;status = "disabled";};- timer@10040000 { compatible = "tesla,fsd-mct", "samsung,exynos4210-
mct"; reg = <0x0 0x10040000 0x0 0x800>; -- 2.17.1
-----Original Message----- From: Alim Akhtar [mailto:alim.akhtar@samsung.com] Sent: 14 October 2022 06:54 PM To: 'Padmanabhan Rajanbabu' p.rajanbabu@samsung.com; lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: RE: [PATCH 5/6] arm64: dts: fsd: Add I2S DAI node for Tesla FSD
-----Original Message----- From: Padmanabhan Rajanbabu [mailto:p.rajanbabu@samsung.com] Sent: Friday, October 14, 2022 3:52 PM To: lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com;
aswani.reddy@samsung.com
Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
Padmanabhan
Rajanbabu p.rajanbabu@samsung.com Subject: [PATCH 5/6] arm64: dts: fsd: Add I2S DAI node for Tesla FSD
Add device tree node for I2S0 and I2S1 CPU DAI instances for Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
arch/arm64/boot/dts/tesla/fsd-evb.dts | 8 +++++ arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 14 ++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 38 ++++++++++++++++++++++ 3 files changed, 60 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index 1db6ddf03f01..c0a4509499ab 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -41,3 +41,11 @@ &ufs { status = "okay"; };
+&tdm_0 {
- status = "okay";
+};
+&tdm_1 {
- status = "okay";
+}; diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi index e3852c946352..ff6f5d4b16dd 100644 --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi @@ -339,6 +339,20 @@ samsung,pin-pud = <FSD_PIN_PULL_UP>; samsung,pin-drv = <FSD_PIN_DRV_LV4>; };
- i2s0_bus: i2s0-bus {
samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2", "gpd1-3","gpd1-4";
samsung,pin-function = <FSD_PIN_FUNC_2>;samsung,pin-pud = <FSD_PIN_PULL_DOWN>;samsung,pin-drv = <FSD_PIN_DRV_LV4>;- };
- i2s1_bus: i2s1-bus {
samsung,pins = "gpd2-0", "gpd2-1", "gpd2-2", "gpd2-3","gpd2-4";
samsung,pin-function = <FSD_PIN_FUNC_2>;samsung,pin-pud = <FSD_PIN_PULL_DOWN>;samsung,pin-drv = <FSD_PIN_DRV_LV4>;- };
};
&pinctrl_pmu { diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index f35bc5a288c2..5decad45a1b6 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -32,6 +32,8 @@ spi0 = &spi_0; spi1 = &spi_1; spi2 = &spi_2;
tdm0 = &tdm_0;tdm1 = &tdm_1;};
cpus {
@@ -809,6 +811,42 @@ status = "disabled"; };
tdm_0: tdm@140e0000 {compatible = "samsung,exynos7-i2s";reg = <0x0 0x140E0000 0x0 0x100>;Address should be all in small caps Make sure you have run 'make dtbs_check'
Okay
interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;dmas = <&pdma1 14>, <&pdma1 13>, <&pdma1 12>;dma-names = "tx", "rx", "tx-sec";#clock-cells = <1>;#sound-dai-cells = <1>;clocks = <&clock_peric PERIC_HCLK_TDM0>,<&clock_peric PERIC_HCLK_TDM0>,<&clock_peric PERIC_PCLK_TDM0>;clock-names = "i2s_opclk0", "i2s_opclk1", "iis";pinctrl-names = "default";pinctrl-0 = <&i2s0_bus>;samsung,sec-dai-id = <0>;status = "disabled";};tdm_1: tdm@140f0000 {compatible = "samsung,exynos7-i2s";reg = <0x0 0x140F0000 0x0 0x100>;Same as above
Okay
interrupts = <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>;dmas = <&pdma1 17>, <&pdma1 16>, <&pdma1 15>;dma-names = "tx", "rx", "tx-sec";#clock-cells = <1>;#sound-dai-cells = <1>;clocks = <&clock_peric PERIC_HCLK_TDM1>,<&clock_peric PERIC_HCLK_TDM1>,<&clock_peric PERIC_PCLK_TDM1>;clock-names = "i2s_opclk0", "i2s_opclk1", "iis";pinctrl-names = "default";pinctrl-0 = <&i2s1_bus>;samsung,sec-dai-id = <1>;status = "disabled";};- timer@10040000 { compatible = "tesla,fsd-mct", "samsung,exynos4210-
mct";
reg = <0x0 0x10040000 0x0 0x800>;-- 2.17.1
On 14/10/2022 06:21, Padmanabhan Rajanbabu wrote:
Add device tree node for I2S0 and I2S1 CPU DAI instances for Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
arch/arm64/boot/dts/tesla/fsd-evb.dts | 8 +++++ arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 14 ++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 38 ++++++++++++++++++++++ 3 files changed, 60 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index 1db6ddf03f01..c0a4509499ab 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -41,3 +41,11 @@ &ufs { status = "okay"; };
+&tdm_0 {
Alphabetical order against other label-overrides.
- status = "okay";
+};
+&tdm_1 {
- status = "okay";
+}; diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi index e3852c946352..ff6f5d4b16dd 100644 --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi @@ -339,6 +339,20 @@ samsung,pin-pud = <FSD_PIN_PULL_UP>; samsung,pin-drv = <FSD_PIN_DRV_LV4>; };
- i2s0_bus: i2s0-bus {
Does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions).
samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2", "gpd1-3", "gpd1-4";samsung,pin-function = <FSD_PIN_FUNC_2>;samsung,pin-pud = <FSD_PIN_PULL_DOWN>;samsung,pin-drv = <FSD_PIN_DRV_LV4>;- };
- i2s1_bus: i2s1-bus {
samsung,pins = "gpd2-0", "gpd2-1", "gpd2-2", "gpd2-3", "gpd2-4";samsung,pin-function = <FSD_PIN_FUNC_2>;samsung,pin-pud = <FSD_PIN_PULL_DOWN>;samsung,pin-drv = <FSD_PIN_DRV_LV4>;- };
};
&pinctrl_pmu { diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index f35bc5a288c2..5decad45a1b6 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -32,6 +32,8 @@ spi0 = &spi_0; spi1 = &spi_1; spi2 = &spi_2;
tdm0 = &tdm_0;tdm1 = &tdm_1;
Why?
};
cpus { @@ -809,6 +811,42 @@ status = "disabled"; };
tdm_0: tdm@140e0000 {
Node names should be generic, so this looks like i2s. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetre...
compatible = "samsung,exynos7-i2s";reg = <0x0 0x140E0000 0x0 0x100>;interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;dmas = <&pdma1 14>, <&pdma1 13>, <&pdma1 12>;dma-names = "tx", "rx", "tx-sec";#clock-cells = <1>;#sound-dai-cells = <1>;clocks = <&clock_peric PERIC_HCLK_TDM0>,<&clock_peric PERIC_HCLK_TDM0>,<&clock_peric PERIC_PCLK_TDM0>;clock-names = "i2s_opclk0", "i2s_opclk1", "iis";
Does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions).
Best regards, Krzysztof
-----Original Message----- From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] Sent: 16 October 2022 08:44 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com; lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 5/6] arm64: dts: fsd: Add I2S DAI node for Tesla FSD
On 14/10/2022 06:21, Padmanabhan Rajanbabu wrote:
Add device tree node for I2S0 and I2S1 CPU DAI instances for Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
arch/arm64/boot/dts/tesla/fsd-evb.dts | 8 +++++ arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 14 ++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 38 ++++++++++++++++++++++ 3 files changed, 60 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index 1db6ddf03f01..c0a4509499ab 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -41,3 +41,11 @@ &ufs { status = "okay"; };
+&tdm_0 {
Alphabetical order against other label-overrides.
Okay
- status = "okay";
+};
+&tdm_1 {
- status = "okay";
+}; diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi index e3852c946352..ff6f5d4b16dd 100644 --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi @@ -339,6 +339,20 @@ samsung,pin-pud = <FSD_PIN_PULL_UP>; samsung,pin-drv = <FSD_PIN_DRV_LV4>; };
- i2s0_bus: i2s0-bus {
Does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions).
I'll double check and run dtbs_check to see if I'm hitting any errors.
samsung,pins = "gpd1-0", "gpd1-1", "gpd1-2", "gpd1-3","gpd1-4";
samsung,pin-function = <FSD_PIN_FUNC_2>;samsung,pin-pud = <FSD_PIN_PULL_DOWN>;samsung,pin-drv = <FSD_PIN_DRV_LV4>;- };
- i2s1_bus: i2s1-bus {
samsung,pins = "gpd2-0", "gpd2-1", "gpd2-2", "gpd2-3","gpd2-4";
samsung,pin-function = <FSD_PIN_FUNC_2>;samsung,pin-pud = <FSD_PIN_PULL_DOWN>;samsung,pin-drv = <FSD_PIN_DRV_LV4>;- };
};
&pinctrl_pmu { diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index f35bc5a288c2..5decad45a1b6 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -32,6 +32,8 @@ spi0 = &spi_0; spi1 = &spi_1; spi2 = &spi_2;
tdm0 = &tdm_0;tdm1 = &tdm_1;Why?
Sorry, these aliases are not used right now. I'll remove it.
};
cpus { @@ -809,6 +811,42 @@ status = "disabled"; };
tdm_0: tdm@140e0000 {Node names should be generic, so this looks like i2s. https://protect2.fireeye.com/v1/url?k=2cfaa5af-4d874de8-2cfb2ee0- 74fe485fff30-cb16acc0c0c574e9&q=1&e=fc8e3b54-a0ef-475e-a4f2- 83626a86ac8a&u=https%3A%2F%2Fdevicetree- specification.readthedocs.io%2Fen%2Flatest%2Fchapter2-devicetree- basics.html%23generic-names-recommendation
Thank you for the link. I could only find audio-controller in the list and not i2s. so I believe I can use audio-controller node name. Please correct me otherwise.
compatible = "samsung,exynos7-i2s";reg = <0x0 0x140E0000 0x0 0x100>;interrupts = <GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>;dmas = <&pdma1 14>, <&pdma1 13>, <&pdma1 12>;dma-names = "tx", "rx", "tx-sec";#clock-cells = <1>;#sound-dai-cells = <1>;clocks = <&clock_peric PERIC_HCLK_TDM0>,<&clock_peric PERIC_HCLK_TDM0>,<&clock_peric PERIC_PCLK_TDM0>;clock-names = "i2s_opclk0", "i2s_opclk1", "iis";Does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions).
I'll double check and run dtbs_check to see if I'm hitting any errors.
Best regards, Krzysztof
Thank you for reviewing the patch
On 21/10/2022 04:49, Padmanabhan Rajanbabu wrote:
cpus { @@ -809,6 +811,42 @@ status = "disabled"; };
tdm_0: tdm@140e0000 {Node names should be generic, so this looks like i2s. https://protect2.fireeye.com/v1/url?k=2cfaa5af-4d874de8-2cfb2ee0- 74fe485fff30-cb16acc0c0c574e9&q=1&e=fc8e3b54-a0ef-475e-a4f2- 83626a86ac8a&u=https%3A%2F%2Fdevicetree- specification.readthedocs.io%2Fen%2Flatest%2Fchapter2-devicetree- basics.html%23generic-names-recommendation
Thank you for the link. I could only find audio-controller in the list and not i2s. so I believe I can use audio-controller node name. Please correct me otherwise.
All I2S controllers use node name "i2s", so if this is I2S, then use "i2s".
Best regards, Krzysztof
-----Original Message----- From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] Sent: 21 October 2022 06:32 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com; lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 5/6] arm64: dts: fsd: Add I2S DAI node for Tesla FSD
On 21/10/2022 04:49, Padmanabhan Rajanbabu wrote:
cpus { @@ -809,6 +811,42 @@ status = "disabled"; };
tdm_0: tdm@140e0000 {Node names should be generic, so this looks like i2s. https://protect2.fireeye.com/v1/url?k=2cfaa5af-4d874de8-2cfb2ee0- 74fe485fff30-cb16acc0c0c574e9&q=1&e=fc8e3b54-a0ef-475e-a4f2- 83626a86ac8a&u=https%3A%2F%2Fdevicetree- specification.readthedocs.io%2Fen%2Flatest%2Fchapter2-devicetree- basics.html%23generic-names-recommendation
Thank you for the link. I could only find audio-controller in the list and not i2s. so I believe I can use audio-controller node name. Please correct me otherwise.
All I2S controllers use node name "i2s", so if this is I2S, then use "i2s".
Okay, I'll ensure the next version will use "i2s" node name
Best regards, Krzysztof
Add device tree node support for sound card on Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com --- arch/arm64/boot/dts/tesla/fsd-evb.dts | 49 +++++++++++++++++++++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 3 ++ 2 files changed, 52 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index c0a4509499ab..ecaa3c2e3045 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -49,3 +49,52 @@ &tdm_1 { status = "okay"; }; + +&sound { + compatible = "tesla,fsd-sndcard"; + status = "okay"; + model = "fsd-i2s"; + widgets = + "Speaker", "MAIN SPK", + "Microphone", "MAIN MIC"; + + primary-dai-link-0 { + link-name = "fsd-primary-0"; + dai-format = "i2s"; + tesla,bitclock-master = <&tdm_0>; + tesla,frame-master = <&tdm_0>; + cpu { + sound-dai = <&tdm_0 0>; + }; + }; + + secondary-dai-link-0 { + link-name = "fsd-secondary-0"; + dai-format = "i2s"; + tesla,bitclock-master = <&tdm_0>; + tesla,frame-master = <&tdm_0>; + cpu { + sound-dai = <&tdm_0 1>; + }; + }; + + primary-dai-link-1 { + link-name = "fsd-primary-1"; + dai-format = "i2s"; + tesla,bitclock-master = <&tdm_1>; + tesla,frame-master = <&tdm_1>; + cpu { + sound-dai = <&tdm_1 0>; + }; + }; + + secondary-dai-link-1 { + link-name = "fsd-secondary-1"; + dai-format = "i2s"; + tesla,bitclock-master = <&tdm_1>; + tesla,frame-master = <&tdm_1>; + cpu { + sound-dai = <&tdm_1 1>; + }; + }; +}; diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index 5decad45a1b6..fc8931f830a7 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -847,6 +847,9 @@ status = "disabled"; };
+ sound: sound { + }; + timer@10040000 { compatible = "tesla,fsd-mct", "samsung,exynos4210-mct"; reg = <0x0 0x10040000 0x0 0x800>;
-----Original Message----- From: Padmanabhan Rajanbabu [mailto:p.rajanbabu@samsung.com] Sent: Friday, October 14, 2022 3:52 PM To: lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org; Padmanabhan Rajanbabu p.rajanbabu@samsung.com Subject: [PATCH 6/6] arm64: dts: fsd: Add sound card node for Tesla FSD
Add device tree node support for sound card on Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
arch/arm64/boot/dts/tesla/fsd-evb.dts | 49 +++++++++++++++++++++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 3 ++ 2 files changed, 52 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index c0a4509499ab..ecaa3c2e3045 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -49,3 +49,52 @@ &tdm_1 { status = "okay"; };
+&sound {
- compatible = "tesla,fsd-sndcard";
- status = "okay";
- model = "fsd-i2s";
- widgets =
"Speaker", "MAIN SPK","Microphone", "MAIN MIC";- primary-dai-link-0 {
link-name = "fsd-primary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 0>;};- };
- secondary-dai-link-0 {
link-name = "fsd-secondary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 1>;};- };
- primary-dai-link-1 {
link-name = "fsd-primary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 0>;};- };
- secondary-dai-link-1 {
link-name = "fsd-secondary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 1>;};- };
+}; diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index 5decad45a1b6..fc8931f830a7 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -847,6 +847,9 @@ status = "disabled"; };
sound: sound {};
Why to have an empty node in dtsi?
timer@10040000 { compatible = "tesla,fsd-mct", "samsung,exynos4210-mct"; reg = <0x0 0x10040000 0x0 0x800>; -- 2.17.1
-----Original Message----- From: Alim Akhtar [mailto:alim.akhtar@samsung.com] Sent: 14 October 2022 06:59 PM To: 'Padmanabhan Rajanbabu' p.rajanbabu@samsung.com; lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: RE: [PATCH 6/6] arm64: dts: fsd: Add sound card node for Tesla FSD
-----Original Message----- From: Padmanabhan Rajanbabu [mailto:p.rajanbabu@samsung.com] Sent: Friday, October 14, 2022 3:52 PM To: lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com;
aswani.reddy@samsung.com
Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
Padmanabhan
Rajanbabu p.rajanbabu@samsung.com Subject: [PATCH 6/6] arm64: dts: fsd: Add sound card node for Tesla FSD
Add device tree node support for sound card on Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
arch/arm64/boot/dts/tesla/fsd-evb.dts | 49 +++++++++++++++++++++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 3 ++ 2 files changed, 52 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index c0a4509499ab..ecaa3c2e3045 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -49,3 +49,52 @@ &tdm_1 { status = "okay"; };
+&sound {
- compatible = "tesla,fsd-sndcard";
- status = "okay";
- model = "fsd-i2s";
- widgets =
"Speaker", "MAIN SPK","Microphone", "MAIN MIC";- primary-dai-link-0 {
link-name = "fsd-primary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 0>;};- };
- secondary-dai-link-0 {
link-name = "fsd-secondary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 1>;};- };
- primary-dai-link-1 {
link-name = "fsd-primary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 0>;};- };
- secondary-dai-link-1 {
link-name = "fsd-secondary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 1>;};- };
+}; diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index 5decad45a1b6..fc8931f830a7 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -847,6 +847,9 @@ status = "disabled"; };
sound: sound {};Why to have an empty node in dtsi?
This is required as every node we use in dts should have the same declared in dtsi. Sound nodes in most of the platform is only declared (dummy node) in dtsi and defining only in dts. Thus we are following the same.
timer@10040000 { compatible = "tesla,fsd-mct", "samsung,exynos4210-mct";
reg = <0x0 0x10040000 0x0 0x800>;-- 2.17.1
Thank you for reviewing the patch
On 21/10/2022 04:12, Padmanabhan Rajanbabu wrote:
-----Original Message----- From: Alim Akhtar [mailto:alim.akhtar@samsung.com] Sent: 14 October 2022 06:59 PM To: 'Padmanabhan Rajanbabu' p.rajanbabu@samsung.com; lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: RE: [PATCH 6/6] arm64: dts: fsd: Add sound card node for Tesla FSD
-----Original Message----- From: Padmanabhan Rajanbabu [mailto:p.rajanbabu@samsung.com] Sent: Friday, October 14, 2022 3:52 PM To: lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com;
aswani.reddy@samsung.com
Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
Padmanabhan
Rajanbabu p.rajanbabu@samsung.com Subject: [PATCH 6/6] arm64: dts: fsd: Add sound card node for Tesla FSD
Add device tree node support for sound card on Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
arch/arm64/boot/dts/tesla/fsd-evb.dts | 49 +++++++++++++++++++++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 3 ++ 2 files changed, 52 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index c0a4509499ab..ecaa3c2e3045 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -49,3 +49,52 @@ &tdm_1 { status = "okay"; };
+&sound {
- compatible = "tesla,fsd-sndcard";
- status = "okay";
- model = "fsd-i2s";
- widgets =
"Speaker", "MAIN SPK","Microphone", "MAIN MIC";- primary-dai-link-0 {
link-name = "fsd-primary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 0>;};- };
- secondary-dai-link-0 {
link-name = "fsd-secondary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 1>;};- };
- primary-dai-link-1 {
link-name = "fsd-primary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 0>;};- };
- secondary-dai-link-1 {
link-name = "fsd-secondary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 1>;};- };
+}; diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index 5decad45a1b6..fc8931f830a7 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -847,6 +847,9 @@ status = "disabled"; };
sound: sound {};Why to have an empty node in dtsi?
This is required as every node we use in dts should have the same declared in
I see no reason why this is required.
dtsi. Sound nodes in most of the platform is only declared (dummy node) in dtsi and defining only in dts. Thus we are following the same.
Can you point me to Samsung platform doing this?
Keep the code consistent with Exynos style.
Best regards, Krzysztof
-----Original Message----- From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] Sent: 21 October 2022 06:24 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com; 'Alim Akhtar' alim.akhtar@samsung.com; lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: Re: [PATCH 6/6] arm64: dts: fsd: Add sound card node for Tesla FSD
On 21/10/2022 04:12, Padmanabhan Rajanbabu wrote:
-----Original Message----- From: Alim Akhtar [mailto:alim.akhtar@samsung.com] Sent: 14 October 2022 06:59 PM To: 'Padmanabhan Rajanbabu' p.rajanbabu@samsung.com; lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org Subject: RE: [PATCH 6/6] arm64: dts: fsd: Add sound card node for Tesla FSD
-----Original Message----- From: Padmanabhan Rajanbabu [mailto:p.rajanbabu@samsung.com] Sent: Friday, October 14, 2022 3:52 PM To: lgirdwood@gmail.com; broonie@kernel.org; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com;
aswani.reddy@samsung.com
Cc: alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
Padmanabhan
Rajanbabu p.rajanbabu@samsung.com Subject: [PATCH 6/6] arm64: dts: fsd: Add sound card node for Tesla FSD
Add device tree node support for sound card on Tesla FSD board
Signed-off-by: Padmanabhan Rajanbabu p.rajanbabu@samsung.com
arch/arm64/boot/dts/tesla/fsd-evb.dts | 49 +++++++++++++++++++++++++++ arch/arm64/boot/dts/tesla/fsd.dtsi | 3 ++ 2 files changed, 52 insertions(+)
diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts index c0a4509499ab..ecaa3c2e3045 100644 --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts @@ -49,3 +49,52 @@ &tdm_1 { status = "okay"; };
+&sound {
- compatible = "tesla,fsd-sndcard";
- status = "okay";
- model = "fsd-i2s";
- widgets =
"Speaker", "MAIN SPK","Microphone", "MAIN MIC";- primary-dai-link-0 {
link-name = "fsd-primary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 0>;};- };
- secondary-dai-link-0 {
link-name = "fsd-secondary-0";dai-format = "i2s";tesla,bitclock-master = <&tdm_0>;tesla,frame-master = <&tdm_0>;cpu {sound-dai = <&tdm_0 1>;};- };
- primary-dai-link-1 {
link-name = "fsd-primary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 0>;};- };
- secondary-dai-link-1 {
link-name = "fsd-secondary-1";dai-format = "i2s";tesla,bitclock-master = <&tdm_1>;tesla,frame-master = <&tdm_1>;cpu {sound-dai = <&tdm_1 1>;};- };
+}; diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi index 5decad45a1b6..fc8931f830a7 100644 --- a/arch/arm64/boot/dts/tesla/fsd.dtsi +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi @@ -847,6 +847,9 @@ status = "disabled"; };
sound: sound {};Why to have an empty node in dtsi?
This is required as every node we use in dts should have the same declared in
I see no reason why this is required.
dtsi. Sound nodes in most of the platform is only declared (dummy node) in dtsi and defining only in dts. Thus we are following the same.
Can you point me to Samsung platform doing this?
Keep the code consistent with Exynos style.
Okay, will add the sound node in accordance with Exynos style
Best regards, Krzysztof
participants (5)
-
Alim Akhtar -
Krzysztof Kozlowski -
Mark Brown -
Padmanabhan Rajanbabu -
Rob Herring