| Message ID | 20230802084301.134122-1-xingyu.wu@starfivetech.com |
|---|---|
| Headers | show |
| Series | Add I2S support for the StarFive JH7110 SoC | expand |
On 02.08.2023 11:42, Xingyu Wu wrote: > diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c > index c076090a9864..4dfbd8ddbcf5 100644 > --- a/sound/soc/dwc/dwc-i2s.c > +++ b/sound/soc/dwc/dwc-i2s.c > @@ -16,6 +16,7 @@ > #include <linux/init.h> > #include <linux/io.h> > #include <linux/interrupt.h> > +#include <linux/mfd/syscon.h> > #include <linux/module.h> > #include <linux/reset.h> > #include <linux/slab.h> > @@ -198,7 +199,7 @@ static void i2s_start(struct dw_i2s_dev *dev, > else > i2s_write_reg(dev->i2s_base, IRER, 1); > > - if (dev->use_pio) > + if (dev->use_pio || dev->is_jh7110) > i2s_enable_irqs(dev, substream->stream, config->chan_nr); > else > i2s_enable_dma(dev, substream->stream); > @@ -216,7 +217,7 @@ static void i2s_stop(struct dw_i2s_dev *dev, > else > i2s_write_reg(dev->i2s_base, IRER, 0); > > - if (dev->use_pio) > + if (dev->use_pio || dev->is_jh7110) > i2s_disable_irqs(dev, substream->stream, 8); > else > i2s_disable_dma(dev, substream->stream); Why do we need to enable interrupts for DMA mode? > @@ -227,6 +228,21 @@ static void i2s_stop(struct dw_i2s_dev *dev, > } > } > > +static int dw_i2s_startup(struct snd_pcm_substream *substream, > + struct snd_soc_dai *cpu_dai) > +{ > + struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai); > + > + if (dev->is_jh7110) { > + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); > + struct snd_soc_dai_link *dai_link = rtd->dai_link; > + > + dai_link->trigger_stop = SND_SOC_TRIGGER_ORDER_LDC; > + } > + > + return 0; > +} > + > static void dw_i2s_config(struct dw_i2s_dev *dev, int stream) > { > u32 ch_reg; > @@ -267,6 +283,11 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, > config->data_width = 16; > dev->ccr = 0x00; > dev->xfer_resolution = 0x02; > + /* Set DMA buswidth on JH7110 */ > + if (dev->is_jh7110) { > + dev->play_dma_data.dt.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; > + dev->capture_dma_data.dt.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; > + } Not needed. See: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20230802&id=6f80197f40515853814d0f22e5209d53f899ab91 Proper bus width calculations is performed by snd_hwparams_to_dma_slave_config() > break; > > case SNDRV_PCM_FORMAT_S24_LE: > @@ -279,6 +300,11 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, > config->data_width = 32; > dev->ccr = 0x10; > dev->xfer_resolution = 0x05; > + /* Set DMA buswidth on JH7110 */ > + if (dev->is_jh7110) { > + dev->play_dma_data.dt.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; > + dev->capture_dma_data.dt.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; > + } Not needed.
On 2023/8/3 13:16, Maxim Kochetkov wrote: > > > On 02.08.2023 11:42, Xingyu Wu wrote: > >> diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c >> index c076090a9864..4dfbd8ddbcf5 100644 >> --- a/sound/soc/dwc/dwc-i2s.c >> +++ b/sound/soc/dwc/dwc-i2s.c >> @@ -16,6 +16,7 @@ >> #include <linux/init.h> >> #include <linux/io.h> >> #include <linux/interrupt.h> >> +#include <linux/mfd/syscon.h> >> #include <linux/module.h> >> #include <linux/reset.h> >> #include <linux/slab.h> >> @@ -198,7 +199,7 @@ static void i2s_start(struct dw_i2s_dev *dev, >> else >> i2s_write_reg(dev->i2s_base, IRER, 1); >> - if (dev->use_pio) >> + if (dev->use_pio || dev->is_jh7110) >> i2s_enable_irqs(dev, substream->stream, config->chan_nr); >> else >> i2s_enable_dma(dev, substream->stream); >> @@ -216,7 +217,7 @@ static void i2s_stop(struct dw_i2s_dev *dev, >> else >> i2s_write_reg(dev->i2s_base, IRER, 0); >> - if (dev->use_pio) >> + if (dev->use_pio || dev->is_jh7110) >> i2s_disable_irqs(dev, substream->stream, 8); >> else >> i2s_disable_dma(dev, substream->stream); > > Why do we need to enable interrupts for DMA mode? This is special mechanism about data transmission by DMA controller on the StarFive JH7110 SoC. The DMA controller needs to make a handshake with I2S interrupt and cannot move the audio data without enabling the I2S interrupt. So I should keep the I2S interrupt enabled in both interrupt or DMA mode. > >> @@ -227,6 +228,21 @@ static void i2s_stop(struct dw_i2s_dev *dev, >> } >> } >> +static int dw_i2s_startup(struct snd_pcm_substream *substream, >> + struct snd_soc_dai *cpu_dai) >> +{ >> + struct dw_i2s_dev *dev = snd_soc_dai_get_drvdata(cpu_dai); >> + >> + if (dev->is_jh7110) { >> + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); >> + struct snd_soc_dai_link *dai_link = rtd->dai_link; >> + >> + dai_link->trigger_stop = SND_SOC_TRIGGER_ORDER_LDC; >> + } >> + >> + return 0; >> +} >> + >> static void dw_i2s_config(struct dw_i2s_dev *dev, int stream) >> { >> u32 ch_reg; >> @@ -267,6 +283,11 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, >> config->data_width = 16; >> dev->ccr = 0x00; >> dev->xfer_resolution = 0x02; >> + /* Set DMA buswidth on JH7110 */ >> + if (dev->is_jh7110) { >> + dev->play_dma_data.dt.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; >> + dev->capture_dma_data.dt.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES; >> + } > > Not needed. > See: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20230802&id=6f80197f40515853814d0f22e5209d53f899ab91 > Proper bus width calculations is performed by snd_hwparams_to_dma_slave_config() Will use this ops instead. > >> break; >> case SNDRV_PCM_FORMAT_S24_LE: >> @@ -279,6 +300,11 @@ static int dw_i2s_hw_params(struct snd_pcm_substream *substream, >> config->data_width = 32; >> dev->ccr = 0x10; >> dev->xfer_resolution = 0x05; >> + /* Set DMA buswidth on JH7110 */ >> + if (dev->is_jh7110) { >> + dev->play_dma_data.dt.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; >> + dev->capture_dma_data.dt.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; >> + } > > Not needed Will drop. > Thanks, Xingyu Wu
On 02/08/2023 10:43, Xingyu Wu wrote: > Add I2Srx/I2Stx0/I2Stx1 nodes and pins configuration for the > StarFive JH7110 SoC. > > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > --- ... > + > spi0_pins: spi0-0 { > mosi-pins { > pinmux = <GPIOMUX(52, GPOUT_SYS_SPI0_TXD, > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi > index 05f843b8ca03..507312eb6053 100644 > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi > @@ -512,6 +512,30 @@ tdm: tdm@10090000 { > status = "disabled"; > }; > > + i2srx: i2srx@100e0000 { Node names should be generic, so: i2s@ > + compatible = "starfive,jh7110-i2srx"; > + reg = <0x0 0x100e0000 0x0 0x1000>; > + clocks = <&syscrg JH7110_SYSCLK_I2SRX_BCLK_MST>, > + <&syscrg JH7110_SYSCLK_I2SRX_APB>, > + <&syscrg JH7110_SYSCLK_MCLK>, > + <&syscrg JH7110_SYSCLK_MCLK_INNER>, > + <&mclk_ext>, > + <&syscrg JH7110_SYSCLK_I2SRX_BCLK>, > + <&syscrg JH7110_SYSCLK_I2SRX_LRCK>, > + <&i2srx_bclk_ext>, > + <&i2srx_lrck_ext>; > + clock-names = "i2sclk", "apb", "mclk", > + "mclk_inner", "mclk_ext", "bclk", > + "lrck", "bclk_ext", "lrck_ext"; > + resets = <&syscrg JH7110_SYSRST_I2SRX_APB>, > + <&syscrg JH7110_SYSRST_I2SRX_BCLK>; > + dmas = <0>, <&dma 24>; > + dma-names = "tx", "rx"; > + starfive,syscon = <&sys_syscon 0x18 0x2>; > + #sound-dai-cells = <0>; > + status = "disabled"; > + }; > + > usb0: usb@10100000 { > compatible = "starfive,jh7110-usb"; > ranges = <0x0 0x0 0x10100000 0x100000>; > @@ -736,6 +760,47 @@ spi6: spi@120a0000 { > status = "disabled"; > }; > > + i2stx0: i2stx0@120b0000 { i2s@ > + compatible = "starfive,jh7110-i2stx0"; > + reg = <0x0 0x120b0000 0x0 0x1000>; > + clocks = <&syscrg JH7110_SYSCLK_I2STX0_BCLK_MST>, > + <&syscrg JH7110_SYSCLK_I2STX0_APB>, > + <&syscrg JH7110_SYSCLK_MCLK>, > + <&syscrg JH7110_SYSCLK_MCLK_INNER>, > + <&mclk_ext>; > + clock-names = "i2sclk", "apb", "mclk", > + "mclk_inner","mclk_ext"; > + resets = <&syscrg JH7110_SYSRST_I2STX0_APB>, > + <&syscrg JH7110_SYSRST_I2STX0_BCLK>; > + dmas = <&dma 47>; > + dma-names = "tx"; > + #sound-dai-cells = <0>; > + status = "disabled"; > + }; > + > + i2stx1: i2stx1@120c0000 { i2s@ Best regards, Krzysztof
On 2023/8/6 5:04, Krzysztof Kozlowski wrote: > On 02/08/2023 10:43, Xingyu Wu wrote: >> Add I2Srx/I2Stx0/I2Stx1 nodes and pins configuration for the >> StarFive JH7110 SoC. >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> --- > > ... > >> + >> spi0_pins: spi0-0 { >> mosi-pins { >> pinmux = <GPIOMUX(52, GPOUT_SYS_SPI0_TXD, >> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> index 05f843b8ca03..507312eb6053 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi >> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> @@ -512,6 +512,30 @@ tdm: tdm@10090000 { >> status = "disabled"; >> }; >> >> + i2srx: i2srx@100e0000 { > > Node names should be generic, so: > i2s@ Will fix. Thanks. > >> + compatible = "starfive,jh7110-i2srx"; >> + reg = <0x0 0x100e0000 0x0 0x1000>; >> + clocks = <&syscrg JH7110_SYSCLK_I2SRX_BCLK_MST>, >> + <&syscrg JH7110_SYSCLK_I2SRX_APB>, >> + <&syscrg JH7110_SYSCLK_MCLK>, >> + <&syscrg JH7110_SYSCLK_MCLK_INNER>, >> + <&mclk_ext>, >> + <&syscrg JH7110_SYSCLK_I2SRX_BCLK>, >> + <&syscrg JH7110_SYSCLK_I2SRX_LRCK>, >> + <&i2srx_bclk_ext>, >> + <&i2srx_lrck_ext>; >> + clock-names = "i2sclk", "apb", "mclk", >> + "mclk_inner", "mclk_ext", "bclk", >> + "lrck", "bclk_ext", "lrck_ext"; >> + resets = <&syscrg JH7110_SYSRST_I2SRX_APB>, >> + <&syscrg JH7110_SYSRST_I2SRX_BCLK>; >> + dmas = <0>, <&dma 24>; >> + dma-names = "tx", "rx"; >> + starfive,syscon = <&sys_syscon 0x18 0x2>; >> + #sound-dai-cells = <0>; >> + status = "disabled"; >> + }; >> + >> usb0: usb@10100000 { >> compatible = "starfive,jh7110-usb"; >> ranges = <0x0 0x0 0x10100000 0x100000>; >> @@ -736,6 +760,47 @@ spi6: spi@120a0000 { >> status = "disabled"; >> }; >> >> + i2stx0: i2stx0@120b0000 { > > i2s@ Will fix. > >> + compatible = "starfive,jh7110-i2stx0"; >> + reg = <0x0 0x120b0000 0x0 0x1000>; >> + clocks = <&syscrg JH7110_SYSCLK_I2STX0_BCLK_MST>, >> + <&syscrg JH7110_SYSCLK_I2STX0_APB>, >> + <&syscrg JH7110_SYSCLK_MCLK>, >> + <&syscrg JH7110_SYSCLK_MCLK_INNER>, >> + <&mclk_ext>; >> + clock-names = "i2sclk", "apb", "mclk", >> + "mclk_inner","mclk_ext"; >> + resets = <&syscrg JH7110_SYSRST_I2STX0_APB>, >> + <&syscrg JH7110_SYSRST_I2STX0_BCLK>; >> + dmas = <&dma 47>; >> + dma-names = "tx"; >> + #sound-dai-cells = <0>; >> + status = "disabled"; >> + }; >> + >> + i2stx1: i2stx1@120c0000 { > > i2s@ Will fix. Best regards, Xingyu Wu
This patch series adds I2S support for the StarFive JH7110 RISC-V SoC based on Designware I2S controller. There has three I2S channels (RX/TX0/TX1) on the JH7110 SoC, one of which is for record(RX) and two for playback(TX). The first patch adds the ops to get data from platform bus in the I2S driver. The second patch adds support for the StarFive JH7110 SoC in the Designware I2S bindings. The third patch adds support for the StarFive JH7110 SoC in the Designware I2S driver. The fourth patch fixes the name of I2STX1 pinmux. The last patch adds device node of I2S RX/TX0/TX1 in JH7110 dts. This patch series is based on Linux-next which is merge clock, syscon and dma nodes for the StarFive JH7110 SoC. And these should be applied after the following patchset: https://lore.kernel.org/all/20230724055440.100947-1-xingyu.wu@starfivetech.com/ The series has been tested and works normally on the VisionFive 2 board by plugging an audio expansion board. Xingyu Wu (5): ASoC: dwc: Use ops to get platform data ASoC: dt-bindings: snps,designware-i2s: Add StarFive JH7110 SoC support ASoC: dwc: i2s: Add StarFive JH7110 SoC support riscv: dts: starfive: pinfunc: Fix the pins name of I2STX1 riscv: dts: starfive: Add the nodes and pins of I2Srx/I2Stx0/I2Stx1 .../bindings/sound/snps,designware-i2s.yaml | 101 +++++- arch/riscv/boot/dts/starfive/jh7110-pinfunc.h | 4 +- .../jh7110-starfive-visionfive-2.dtsi | 58 ++++ arch/riscv/boot/dts/starfive/jh7110.dtsi | 65 ++++ include/sound/designware_i2s.h | 3 + sound/soc/dwc/dwc-i2s.c | 318 ++++++++++++++++-- sound/soc/dwc/local.h | 1 + 7 files changed, 523 insertions(+), 27 deletions(-) base-commit: 0fee53365d051781156b75c6f6e6015757e71fd8 prerequisite-patch-id: 0159f09bb0a1ff711a00ae17ef5b12662c9c7d3d