| Message ID | 20230911024023.43833-1-mwkim@gaonchips.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ASoC: dwc: Add Single DMA mode support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 1 warnings, 48 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 11/09/2023 04:40, Myunguk Kim wrote: > There is a SoC between dwc and DMA block (ie. PL330) > that does not have a burst signal and supports only single. > > So write not-support-burst property on dts, it support single DMA mode. > > Signed-off-by: Myunguk Kim <mwkim@gaonchips.com> > --- > .../bindings/sound/snps,designware-i2s.yaml | 3 +++ Bindings are always separate patch. Please run scripts/checkpatch.pl and fix reported warnings. Some warnings can be ignored, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. > include/sound/designware_i2s.h | 1 + > sound/soc/dwc/dwc-i2s.c | 12 ++++++++++-- > 3 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/sound/snps,designware-i2s.yaml b/Documentation/devicetree/bindings/sound/snps,designware-i2s.yaml > index a48d040b0a4f..43a46ba2a70c 100644 > --- a/Documentation/devicetree/bindings/sound/snps,designware-i2s.yaml > +++ b/Documentation/devicetree/bindings/sound/snps,designware-i2s.yaml > @@ -86,6 +86,9 @@ properties: > The phandle to System Register Controller syscon node and the I2S-rx(ADC) > enabled control offset and mask of SYS_SYSCONSAIF__SYSCFG register. > > + no-burst: > + description: Use single transaction only between DesignWare I2S and DMA IP. This was not tested. Missing vendor prefix, type. Anyway please provide some explanation why this cannot be deduced from the compatible. Best regards, Krzysztof
On Mon, Sep 11, 2023 at 11:40:23AM +0900, Myunguk Kim wrote: > There is a SoC between dwc and DMA block (ie. PL330) > that does not have a burst signal and supports only single. > > So write not-support-burst property on dts, it support single DMA mode. This feels like something we ought to be discovering from the DMA API somehow, while it's not quite a property of the DMA controller (but rather of the SoC integration) in this case it could be a property of some DMA controller elsewhere and the whole process of finding and figuring out the properties of the DMA controler is handled by the DMA API.
> Bindings are always separate patch. Okay, I will send v2. > This was not tested. Missing vendor prefix, type. > > Anyway please provide some explanation why this cannot be deduced from > the compatible. This is not dependent on a specific vendor, but is intended to describe the properties of the signal(single/burst request) connection relationship between i2s and dma.
> This feels like something we ought to be discovering from the DMA API > somehow, while it's not quite a property of the DMA controller (but > rather of the SoC integration) in this case it could be a property of > some DMA controller elsewhere and the whole process of finding and > figuring out the properties of the DMA controler is handled by the DMA > API. In this case, it is not used through the DMA API. The connection relationship is as follows. i2s --- pcm_dmaengine --- DMA IP (ie. pl330) So, control was possible only by directly setting the maxburst property. And, I will send v2 (separate code and bindings) Thanks, myunguk
On 13/09/2023 04:53, Myunguk Kim wrote: >> Bindings are always separate patch. > > Okay, I will send v2. > >> This was not tested. Missing vendor prefix, type. >> >> Anyway please provide some explanation why this cannot be deduced from >> the compatible. > > This is not dependent on a specific vendor, > but is intended to describe > the properties of the signal(single/burst request) connection > relationship between i2s and dma. How does this relationship depend on hardware? Best regards, Krzysztof
>> This is not dependent on a specific vendor, >> but is intended to describe >> the properties of the signal(single/burst request) connection >> relationship between i2s and dma. > > How does this relationship depend on hardware? When designing a SoC, it depends on the RTL and Bus connection. My company has two types of configuration SoC: single and burst to meet ASIC customer's requirements. Thanks, myunguk
On 13/09/2023 08:43, Myunguk Kim wrote: >>> This is not dependent on a specific vendor, >>> but is intended to describe >>> the properties of the signal(single/burst request) connection >>> relationship between i2s and dma. >> >> How does this relationship depend on hardware? > > When designing a SoC, it depends on the RTL and Bus connection. > My company has two types of configuration SoC: single and burst > to meet ASIC customer's requirements. Then it is specific to SoC, thus can be deduced from compatible. Best regards, Krzysztof
On Wed, Sep 13, 2023 at 01:09:03PM +0900, Myunguk Kim wrote: > In this case, it is not used through the DMA API. > The connection relationship is as follows. > i2s --- pcm_dmaengine --- DMA IP (ie. pl330) > So, control was possible only by directly setting the maxburst property. pcm_dmaengine is a wrapper binding the DMA API into ASoC...
diff --git a/Documentation/devicetree/bindings/sound/snps,designware-i2s.yaml b/Documentation/devicetree/bindings/sound/snps,designware-i2s.yaml index a48d040b0a4f..43a46ba2a70c 100644 --- a/Documentation/devicetree/bindings/sound/snps,designware-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/snps,designware-i2s.yaml @@ -86,6 +86,9 @@ properties: The phandle to System Register Controller syscon node and the I2S-rx(ADC) enabled control offset and mask of SYS_SYSCONSAIF__SYSCFG register. + no-burst: + description: Use single transaction only between DesignWare I2S and DMA IP. + allOf: - $ref: dai-common.yaml# - if: diff --git a/include/sound/designware_i2s.h b/include/sound/designware_i2s.h index f6803205a9fb..f0207f21f09d 100644 --- a/include/sound/designware_i2s.h +++ b/include/sound/designware_i2s.h @@ -36,6 +36,7 @@ struct i2s_platform_data { #define DW_I2S_QUIRK_COMP_REG_OFFSET (1 << 0) #define DW_I2S_QUIRK_COMP_PARAM1 (1 << 1) #define DW_I2S_QUIRK_16BIT_IDX_OVERRIDE (1 << 2) + #define DW_I2S_QUIRK_NO_BURST (1 << 3) unsigned int quirks; unsigned int i2s_reg_comp1; unsigned int i2s_reg_comp2; diff --git a/sound/soc/dwc/dwc-i2s.c b/sound/soc/dwc/dwc-i2s.c index 5ab1b3eb2d28..71ff894035a4 100644 --- a/sound/soc/dwc/dwc-i2s.c +++ b/sound/soc/dwc/dwc-i2s.c @@ -713,7 +713,10 @@ static int dw_configure_dai_by_dt(struct dw_i2s_dev *dev, dev->play_dma_data.dt.addr = res->start + I2S_TXDMA; dev->play_dma_data.dt.fifo_size = fifo_depth * (fifo_width[idx2]) >> 8; - dev->play_dma_data.dt.maxburst = 16; + if (dev->quirks & DW_I2S_QUIRK_NO_BURST) + dev->play_dma_data.dt.maxburst = 1; + else + dev->play_dma_data.dt.maxburst = 16; } if (COMP1_RX_ENABLED(comp1)) { idx2 = COMP2_RX_WORDSIZE_0(comp2); @@ -722,7 +725,10 @@ static int dw_configure_dai_by_dt(struct dw_i2s_dev *dev, dev->capture_dma_data.dt.addr = res->start + I2S_RXDMA; dev->capture_dma_data.dt.fifo_size = fifo_depth * (fifo_width[idx2] >> 8); - dev->capture_dma_data.dt.maxburst = 16; + if (dev->quirks & DW_I2S_QUIRK_NO_BURST) + dev->capture_dma_data.dt.maxburst = 1; + else + dev->capture_dma_data.dt.maxburst = 16; } return 0; @@ -979,6 +985,8 @@ static int dw_i2s_probe(struct platform_device *pdev) ret = dw_configure_dai_by_pd(dev, dw_i2s_dai, res, pdata); } else { clk_id = "i2sclk"; + if (of_get_property(pdev->dev.of_node, "no-burst", NULL)) + dev->quirks |= DW_I2S_QUIRK_NO_BURST; ret = dw_configure_dai_by_dt(dev, dw_i2s_dai, res); } if (ret < 0)
There is a SoC between dwc and DMA block (ie. PL330) that does not have a burst signal and supports only single. So write not-support-burst property on dts, it support single DMA mode. Signed-off-by: Myunguk Kim <mwkim@gaonchips.com> --- .../bindings/sound/snps,designware-i2s.yaml | 3 +++ include/sound/designware_i2s.h | 1 + sound/soc/dwc/dwc-i2s.c | 12 ++++++++++-- 3 files changed, 14 insertions(+), 2 deletions(-)