| Message ID | 24f69c50925b93afd7a706bd888ee25d27247c78.1583725533.git.shengjiu.wang@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | ASoC: Add new module driver for new ASRC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Mon, Mar 09, 2020 at 11:58:28AM +0800, Shengjiu Wang wrote: > In order to support new EASRC and simplify the code structure, > We decide to share the common structure between them. This bring > a problem that EASRC accept format directly from devicetree, but > ASRC accept width from devicetree. > > In order to align with new ESARC, we add new property fsl,asrc-format. > The fsl,asrc-format can replace the fsl,asrc-width, then driver > can accept format from devicetree, don't need to convert it to > format through width. > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > --- > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > index cb9a25165503..780455cf7f71 100644 > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > @@ -51,6 +51,11 @@ Optional properties: > will be in use as default. Otherwise, the big endian > mode will be in use for all the device registers. > > + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back > + Ends, which can replace the fsl,asrc-width. > + The value is SNDRV_PCM_FORMAT_S16_LE, or > + SNDRV_PCM_FORMAT_S24_LE I am still holding the concern at the DT binding of this format, as it uses values from ASoC header file instead of a dt-binding header file -- not sure if we can do this. Let's wait for Rob's comments.
Hi Rob On Tue, Mar 10, 2020 at 5:20 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > On Mon, Mar 09, 2020 at 11:58:28AM +0800, Shengjiu Wang wrote: > > In order to support new EASRC and simplify the code structure, > > We decide to share the common structure between them. This bring > > a problem that EASRC accept format directly from devicetree, but > > ASRC accept width from devicetree. > > > > In order to align with new ESARC, we add new property fsl,asrc-format. > > The fsl,asrc-format can replace the fsl,asrc-width, then driver > > can accept format from devicetree, don't need to convert it to > > format through width. > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > > --- > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > index cb9a25165503..780455cf7f71 100644 > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > @@ -51,6 +51,11 @@ Optional properties: > > will be in use as default. Otherwise, the big endian > > mode will be in use for all the device registers. > > > > + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back > > + Ends, which can replace the fsl,asrc-width. > > + The value is SNDRV_PCM_FORMAT_S16_LE, or > > + SNDRV_PCM_FORMAT_S24_LE > > I am still holding the concern at the DT binding of this format, > as it uses values from ASoC header file instead of a dt-binding > header file -- not sure if we can do this. Let's wait for Rob's > comments. Could you please share your comments or proposal about Nicolin's concern? best regards wang shengjiu
On Mon, Mar 09, 2020 at 02:19:44PM -0700, Nicolin Chen wrote: > On Mon, Mar 09, 2020 at 11:58:28AM +0800, Shengjiu Wang wrote: > > In order to support new EASRC and simplify the code structure, > > We decide to share the common structure between them. This bring > > a problem that EASRC accept format directly from devicetree, but > > ASRC accept width from devicetree. > > > > In order to align with new ESARC, we add new property fsl,asrc-format. > > The fsl,asrc-format can replace the fsl,asrc-width, then driver > > can accept format from devicetree, don't need to convert it to > > format through width. > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > > --- > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > index cb9a25165503..780455cf7f71 100644 > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > @@ -51,6 +51,11 @@ Optional properties: > > will be in use as default. Otherwise, the big endian > > mode will be in use for all the device registers. > > > > + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back > > + Ends, which can replace the fsl,asrc-width. > > + The value is SNDRV_PCM_FORMAT_S16_LE, or > > + SNDRV_PCM_FORMAT_S24_LE > > I am still holding the concern at the DT binding of this format, > as it uses values from ASoC header file instead of a dt-binding > header file -- not sure if we can do this. Let's wait for Rob's > comments. I assume those are an ABI as well, so it's okay to copy them unless we already have some format definitions for DT. But it does need to be copy in a header under include/dt-bindings/. Rob
On Sat, Mar 21, 2020 at 1:34 AM Rob Herring <robh@kernel.org> wrote: > > On Mon, Mar 09, 2020 at 02:19:44PM -0700, Nicolin Chen wrote: > > On Mon, Mar 09, 2020 at 11:58:28AM +0800, Shengjiu Wang wrote: > > > In order to support new EASRC and simplify the code structure, > > > We decide to share the common structure between them. This bring > > > a problem that EASRC accept format directly from devicetree, but > > > ASRC accept width from devicetree. > > > > > > In order to align with new ESARC, we add new property fsl,asrc-format. > > > The fsl,asrc-format can replace the fsl,asrc-width, then driver > > > can accept format from devicetree, don't need to convert it to > > > format through width. > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > > > --- > > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > index cb9a25165503..780455cf7f71 100644 > > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > @@ -51,6 +51,11 @@ Optional properties: > > > will be in use as default. Otherwise, the big endian > > > mode will be in use for all the device registers. > > > > > > + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back > > > + Ends, which can replace the fsl,asrc-width. > > > + The value is SNDRV_PCM_FORMAT_S16_LE, or > > > + SNDRV_PCM_FORMAT_S24_LE > > > > I am still holding the concern at the DT binding of this format, > > as it uses values from ASoC header file instead of a dt-binding > > header file -- not sure if we can do this. Let's wait for Rob's > > comments. > > I assume those are an ABI as well, so it's okay to copy them unless we > already have some format definitions for DT. But it does need to be copy > in a header under include/dt-bindings/. Thanks for reviewing. seems it is not a good time to add a new header file in include/dt-bindings/ in this patch serial. I will drop this change this time, that still using the "fsl,asrc-width". best regards wang shengjiu
On Fri, Mar 20, 2020 at 11:32:13AM -0600, Rob Herring wrote: > On Mon, Mar 09, 2020 at 02:19:44PM -0700, Nicolin Chen wrote: > > On Mon, Mar 09, 2020 at 11:58:28AM +0800, Shengjiu Wang wrote: > > > In order to support new EASRC and simplify the code structure, > > > We decide to share the common structure between them. This bring > > > a problem that EASRC accept format directly from devicetree, but > > > ASRC accept width from devicetree. > > > > > > In order to align with new ESARC, we add new property fsl,asrc-format. > > > The fsl,asrc-format can replace the fsl,asrc-width, then driver > > > can accept format from devicetree, don't need to convert it to > > > format through width. > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > > > --- > > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > index cb9a25165503..780455cf7f71 100644 > > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > @@ -51,6 +51,11 @@ Optional properties: > > > will be in use as default. Otherwise, the big endian > > > mode will be in use for all the device registers. > > > > > > + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back > > > + Ends, which can replace the fsl,asrc-width. > > > + The value is SNDRV_PCM_FORMAT_S16_LE, or > > > + SNDRV_PCM_FORMAT_S24_LE > > > > I am still holding the concern at the DT binding of this format, > > as it uses values from ASoC header file instead of a dt-binding > > header file -- not sure if we can do this. Let's wait for Rob's > > comments. > > I assume those are an ABI as well, so it's okay to copy them unless we They are defined under include/uapi. So I think we can use them? > already have some format definitions for DT. But it does need to be copy > in a header under include/dt-bindings/. Shengjiu is actually quoting those integral values, rather than those macros, so actually no need copy to include/dt-bindings, yet whoever adds this format property to a new DT would need to look up the value in a header file under include/uapi. I's just wondering if that's okay. Thanks
Hi On Tue, Mar 24, 2020 at 5:22 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > On Fri, Mar 20, 2020 at 11:32:13AM -0600, Rob Herring wrote: > > On Mon, Mar 09, 2020 at 02:19:44PM -0700, Nicolin Chen wrote: > > > On Mon, Mar 09, 2020 at 11:58:28AM +0800, Shengjiu Wang wrote: > > > > In order to support new EASRC and simplify the code structure, > > > > We decide to share the common structure between them. This bring > > > > a problem that EASRC accept format directly from devicetree, but > > > > ASRC accept width from devicetree. > > > > > > > > In order to align with new ESARC, we add new property fsl,asrc-format. > > > > The fsl,asrc-format can replace the fsl,asrc-width, then driver > > > > can accept format from devicetree, don't need to convert it to > > > > format through width. > > > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > > > > --- > > > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++ > > > > 1 file changed, 5 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > > index cb9a25165503..780455cf7f71 100644 > > > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > > @@ -51,6 +51,11 @@ Optional properties: > > > > will be in use as default. Otherwise, the big endian > > > > mode will be in use for all the device registers. > > > > > > > > + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back > > > > + Ends, which can replace the fsl,asrc-width. > > > > + The value is SNDRV_PCM_FORMAT_S16_LE, or > > > > + SNDRV_PCM_FORMAT_S24_LE > > > > > > I am still holding the concern at the DT binding of this format, > > > as it uses values from ASoC header file instead of a dt-binding > > > header file -- not sure if we can do this. Let's wait for Rob's > > > comments. > > > > I assume those are an ABI as well, so it's okay to copy them unless we > > They are defined under include/uapi. So I think we can use them? > > > already have some format definitions for DT. But it does need to be copy > > in a header under include/dt-bindings/. > > Shengjiu is actually quoting those integral values, rather than > those macros, so actually no need copy to include/dt-bindings, > yet whoever adds this format property to a new DT would need to > look up the value in a header file under include/uapi. I's just > wondering if that's okay. > > Thanks Shall I keep this change or drop this change? best regards wang shengjiu
On Tue, Mar 31, 2020 at 10:28:25AM +0800, Shengjiu Wang wrote: > Hi > > On Tue, Mar 24, 2020 at 5:22 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote: > > > > On Fri, Mar 20, 2020 at 11:32:13AM -0600, Rob Herring wrote: > > > On Mon, Mar 09, 2020 at 02:19:44PM -0700, Nicolin Chen wrote: > > > > On Mon, Mar 09, 2020 at 11:58:28AM +0800, Shengjiu Wang wrote: > > > > > In order to support new EASRC and simplify the code structure, > > > > > We decide to share the common structure between them. This bring > > > > > a problem that EASRC accept format directly from devicetree, but > > > > > ASRC accept width from devicetree. > > > > > > > > > > In order to align with new ESARC, we add new property fsl,asrc-format. > > > > > The fsl,asrc-format can replace the fsl,asrc-width, then driver > > > > > can accept format from devicetree, don't need to convert it to > > > > > format through width. > > > > > > > > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> > > > > > --- > > > > > Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++ > > > > > 1 file changed, 5 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > > > index cb9a25165503..780455cf7f71 100644 > > > > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt > > > > > @@ -51,6 +51,11 @@ Optional properties: > > > > > will be in use as default. Otherwise, the big endian > > > > > mode will be in use for all the device registers. > > > > > > > > > > + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back > > > > > + Ends, which can replace the fsl,asrc-width. > > > > > + The value is SNDRV_PCM_FORMAT_S16_LE, or > > > > > + SNDRV_PCM_FORMAT_S24_LE > > > > > > > > I am still holding the concern at the DT binding of this format, > > > > as it uses values from ASoC header file instead of a dt-binding > > > > header file -- not sure if we can do this. Let's wait for Rob's > > > > comments. > > > > > > I assume those are an ABI as well, so it's okay to copy them unless we > > > > They are defined under include/uapi. So I think we can use them? > > > > > already have some format definitions for DT. But it does need to be copy > > > in a header under include/dt-bindings/. > > > > Shengjiu is actually quoting those integral values, rather than > > those macros, so actually no need copy to include/dt-bindings, > > yet whoever adds this format property to a new DT would need to > > look up the value in a header file under include/uapi. I's just > > wondering if that's okay. > > > > Thanks > Shall I keep this change or drop this change? This version of patch defines the format using those two marcos. So what Rob suggested is to copy those defines from uapi header file to dt-bindings folder. But you don't intend to do that? My follow-up mail is to find if using integral values is doable. Yet, not seeing any reply further. I think you can make a choice to send it -- We will see how Rob acks eventually, or not.
diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt index cb9a25165503..780455cf7f71 100644 --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt @@ -51,6 +51,11 @@ Optional properties: will be in use as default. Otherwise, the big endian mode will be in use for all the device registers. + - fsl,asrc-format : Defines a mutual sample format used by DPCM Back + Ends, which can replace the fsl,asrc-width. + The value is SNDRV_PCM_FORMAT_S16_LE, or + SNDRV_PCM_FORMAT_S24_LE + Example: asrc: asrc@2034000 {
In order to support new EASRC and simplify the code structure, We decide to share the common structure between them. This bring a problem that EASRC accept format directly from devicetree, but ASRC accept width from devicetree. In order to align with new ESARC, we add new property fsl,asrc-format. The fsl,asrc-format can replace the fsl,asrc-width, then driver can accept format from devicetree, don't need to convert it to format through width. Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com> --- Documentation/devicetree/bindings/sound/fsl,asrc.txt | 5 +++++ 1 file changed, 5 insertions(+)