| Message ID | b1c922b3496020f611ecd6ea27d262369646d830.1573462647.git.shengjiu.wang@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [V3,1/2] ASoC: dt-bindings: fsl_asrc: add compatible string for imx8qm | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Mon, Nov 11, 2019 at 05:18:23PM +0800, Shengjiu Wang wrote: > There are two asrc module in imx8qm, each module has different > clock configuration, and the DMA type is EDMA. > > So in this patch, we define the new clocks, refine the clock map, > and include struct fsl_asrc_soc_data for different soc usage. > > The EDMA channel is fixed with each dma request, one dma request > corresponding to one dma channel. So we need to request dma > channel with dma request of asrc module. > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> Two small comments inline. Once they are addressed, Acked-by: Nicolin Chen <nicoleotsuka@gmail.com> > --- > changes in v2 > - use !use_edma to wrap code in fsl_asrc_dma > - add Acked-by: Nicolin Chen > > changes in v3 > - remove the acked-by for commit is updated > - read "fsl,asrc-clk-map" property, and update table > clk_map_imx8qm. > > sound/soc/fsl/fsl_asrc.c | 112 ++++++++++++++++++++++++++++------- > sound/soc/fsl/fsl_asrc.h | 64 +++++++++++++++++++- > sound/soc/fsl/fsl_asrc_dma.c | 42 +++++++++---- > 3 files changed, 183 insertions(+), 35 deletions(-) > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c > index a3cfceea7d2f..03de33de8633 100644 > --- a/sound/soc/fsl/fsl_asrc.c > +++ b/sound/soc/fsl/fsl_asrc.c > @@ -964,14 +1001,33 @@ static int fsl_asrc_probe(struct platform_device *pdev) > + } else if (of_device_is_compatible(np, "fsl,imx8qm-asrc")) { > + ret = of_property_read_u32(np, "fsl,asrc-clk-map", > + &map_idx); This seems to fit a single line? > diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c > index d6146de9acd2..f871fdb9d1c6 100644 > --- a/sound/soc/fsl/fsl_asrc_dma.c > +++ b/sound/soc/fsl/fsl_asrc_dma.c > @@ -197,21 +197,37 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component, > + /* > + * For EDMA DEV_TO_DEV channel, we don't need to configure > + * dma_request and dma_request2, we can get dma channel through > + * dma_request_slave_channel directly. > + * Compare with SDMA channel, EDMA channel is bound with dma > + * request event of each peripheral, and it is fixed. Not like SDMA, > + * the channel is allocated dynamically. So when DMA is EDMA, we > + * can only get EDMA channel through dma-names of Front-End device. > + */ Just trying to make it concise :) + /* + * An EDMA DEV_TO_DEV channel is fixed and bound with DMA event of each + * peripheral, unlike SDMA channel that is allocated dynamically. So no + * need to configure dma_request and dma_request2, but get dma_chan via + * dma_request_slave_channel directly with dma name of Front-End device + */
On Mon, Nov 11, 2019 at 05:18:22PM +0800, Shengjiu Wang wrote: > Add compatible string "fsl,imx8qm-asrc" for imx8qm platform. > > There are two asrc modules in imx8qm, the clock mapping is > different for each other, so add new property "fsl,asrc-clk-map" > to distinguish them. What's the clock mapping? > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > --- > changes in v2 > -none > > changes in v3 > -use only one compatible string "fsl,imx8qm-asrc", > -add new property "fsl,asrc-clk-map". > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > index 1d4d9f938689..02edab7cf3e0 100644 > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > @@ -8,7 +8,8 @@ three substreams within totally 10 channels. > > Required properties: > > - - compatible : Contains "fsl,imx35-asrc" or "fsl,imx53-asrc". > + - compatible : Contains "fsl,imx35-asrc", "fsl,imx53-asrc", > + "fsl,imx8qm-asrc". > > - reg : Offset and length of the register set for the device. > > @@ -35,6 +36,13 @@ Required properties: > > - fsl,asrc-width : Defines a mutual sample width used by DPCM Back Ends. > > + - fsl,asrc-clk-map : Defines clock map used in driver. which is required > + by imx8qm/imx8qxp platform > + <0> - select the map for asrc0 in imx8qm > + <1> - select the map for asrc1 in imx8qm > + <2> - select the map for asrc0 in imx8qxp > + <3> - select the map for asrc1 in imx8qxp Is this 4 modes of the h/w or just selecting 1 of 4 settings defined in the driver? How does one decide? This seems strange. imx8qxp should perhaps be a separate compatible. Then you only need 1 of 2 modes... Rob
Hi Rob On Fri, Nov 15, 2019 at 5:14 AM Rob Herring <robh@kernel.org> wrote: > > On Mon, Nov 11, 2019 at 05:18:22PM +0800, Shengjiu Wang wrote: > > Add compatible string "fsl,imx8qm-asrc" for imx8qm platform. > > > > There are two asrc modules in imx8qm, the clock mapping is > > different for each other, so add new property "fsl,asrc-clk-map" > > to distinguish them. > > What's the clock mapping? > The two asrc have different clock source connected to it, also the asrc in other platform, like imx6, has different clock source. We collect all these clock source together, defined an enumerate format structure in driver, so for the asrc in each platform, we need to remap the clock source from the enumerate index to the real connection index in hardware. The range of the enumerate structure is 0-0x30, some index may not be used in this platform, but used in other platform the range of the real connection range is 0-0xf, so we do the remapping for [0, 0x30] to [0, 0xf] > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > > --- > > changes in v2 > > -none > > > > changes in v3 > > -use only one compatible string "fsl,imx8qm-asrc", > > -add new property "fsl,asrc-clk-map". > > > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > index 1d4d9f938689..02edab7cf3e0 100644 > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > @@ -8,7 +8,8 @@ three substreams within totally 10 channels. > > > > Required properties: > > > > - - compatible : Contains "fsl,imx35-asrc" or "fsl,imx53-asrc". > > + - compatible : Contains "fsl,imx35-asrc", "fsl,imx53-asrc", > > + "fsl,imx8qm-asrc". > > > > - reg : Offset and length of the register set for the device. > > > > @@ -35,6 +36,13 @@ Required properties: > > > > - fsl,asrc-width : Defines a mutual sample width used by DPCM Back Ends. > > > > + - fsl,asrc-clk-map : Defines clock map used in driver. which is required > > + by imx8qm/imx8qxp platform > > + <0> - select the map for asrc0 in imx8qm > > + <1> - select the map for asrc1 in imx8qm > > + <2> - select the map for asrc0 in imx8qxp > > + <3> - select the map for asrc1 in imx8qxp > > Is this 4 modes of the h/w or just selecting 1 of 4 settings defined in > the driver? How does one decide? This seems strange. The setting is defined in driver. please see the following definition in driver. This is some kind of hard code, for the asrc0 in imx8qm, we need to set fsl,asrc-clk-map = 0. +/** + * i.MX8QM/i.MX8QXP uses the same map for input and output. + * clk_map_imx8qm[0] is for i.MX8QM asrc0 + * clk_map_imx8qm[1] is for i.MX8QM asrc1 + * clk_map_imx8qm[2] is for i.MX8QXP asrc0 + * clk_map_imx8qm[3] is for i.MX8QXP asrc1 + */ +static unsigned char clk_map_imx8qm[4][ASRC_CLK_MAP_LEN] = { > > imx8qxp should perhaps be a separate compatible. Then you only need 1 of > 2 modes... > Yes, that is an option. If you agree that we can use fsl,asrc-clk-map to distinguish the clock mapping defined in driver, I can do this change that add new compatible string for imx8qxp. Best Regards Wang Shengjiu
diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt index 1d4d9f938689..02edab7cf3e0 100644 --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt @@ -8,7 +8,8 @@ three substreams within totally 10 channels. Required properties: - - compatible : Contains "fsl,imx35-asrc" or "fsl,imx53-asrc". + - compatible : Contains "fsl,imx35-asrc", "fsl,imx53-asrc", + "fsl,imx8qm-asrc". - reg : Offset and length of the register set for the device. @@ -35,6 +36,13 @@ Required properties: - fsl,asrc-width : Defines a mutual sample width used by DPCM Back Ends. + - fsl,asrc-clk-map : Defines clock map used in driver. which is required + by imx8qm/imx8qxp platform + <0> - select the map for asrc0 in imx8qm + <1> - select the map for asrc1 in imx8qm + <2> - select the map for asrc0 in imx8qxp + <3> - select the map for asrc1 in imx8qxp + Optional properties: - big-endian : If this property is absent, the little endian mode
Add compatible string "fsl,imx8qm-asrc" for imx8qm platform. There are two asrc modules in imx8qm, the clock mapping is different for each other, so add new property "fsl,asrc-clk-map" to distinguish them. Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> --- changes in v2 -none changes in v3 -use only one compatible string "fsl,imx8qm-asrc", -add new property "fsl,asrc-clk-map". Documentation/devicetree/bindings/sound/fsl,asrc.txt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)