| Message ID | 1630934854-14086-1-git-send-email-srivasam@codeaurora.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | ASoC: dt-bindings: lpass: add binding headers for digital codecs | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
Quoting Srinivasa Rao Mandadapu (2021-09-06 06:27:34) > Add header defining for lpass internal digital codecs rx,tx and va > dai node id's. > > Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org> > --- > include/dt-bindings/sound/qcom,lpass.h | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/include/dt-bindings/sound/qcom,lpass.h b/include/dt-bindings/sound/qcom,lpass.h > index 7b0b80b..187af45 100644 > --- a/include/dt-bindings/sound/qcom,lpass.h > +++ b/include/dt-bindings/sound/qcom,lpass.h > @@ -10,6 +10,11 @@ > > #define LPASS_DP_RX 5 > > +#define LPASS_CDC_DMA_RX0 6 > +#define LPASS_CDC_DMA_TX3 7 > +#define LPASS_CDC_DMA_VA0 8 > +#define LPASS_MAX_PORTS 9 Do we need LPASS_MAX_PORTS in the binding?
Thanks For Review Stephen!!! On 9/8/2021 1:54 AM, Stephen Boyd wrote: > Quoting Srinivasa Rao Mandadapu (2021-09-06 06:27:34) >> Add header defining for lpass internal digital codecs rx,tx and va >> dai node id's. >> >> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org> >> --- >> include/dt-bindings/sound/qcom,lpass.h | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/include/dt-bindings/sound/qcom,lpass.h b/include/dt-bindings/sound/qcom,lpass.h >> index 7b0b80b..187af45 100644 >> --- a/include/dt-bindings/sound/qcom,lpass.h >> +++ b/include/dt-bindings/sound/qcom,lpass.h >> @@ -10,6 +10,11 @@ >> >> #define LPASS_DP_RX 5 >> >> +#define LPASS_CDC_DMA_RX0 6 >> +#define LPASS_CDC_DMA_TX3 7 >> +#define LPASS_CDC_DMA_VA0 8 >> +#define LPASS_MAX_PORTS 9 > Do we need LPASS_MAX_PORTS in the binding? Yes. based on this creating array of streams in machine driver. So to make upper limit introduced this macro.
On 06/09/2021 14:27, Srinivasa Rao Mandadapu wrote: > +#define LPASS_CDC_DMA_RX0 6 You are only adding RX0, what happens to RX1.. RX7 TBH, skipping other entries for ex CDC_DMA_RX will end up with sparse numbering. Please add all the entries for CDC_DMA_RX and other ports as well. Like: #define LPASS_CDC_DMA_RX0 6 #define LPASS_CDC_DMA_RX1 7 #define LPASS_CDC_DMA_RX2 8 #define LPASS_CDC_DMA_RX3 9 #define LPASS_CDC_DMA_RX4 10 #define LPASS_CDC_DMA_RX5 11 #define LPASS_CDC_DMA_RX6 12 #define LPASS_CDC_DMA_RX7 13 > +#define LPASS_CDC_DMA_TX3 7 > +#define LPASS_CDC_DMA_VA0 8 You mean VA_TX0? > +#define LPASS_MAX_PORTS 9 We really do not need this in bindings. You could add this is some of the driver header files instead. --srini > +
On Wed, Sep 08, 2021 at 10:08:33AM +0530, Srinivasa Rao Mandadapu wrote: > On 9/8/2021 1:54 AM, Stephen Boyd wrote: > > Quoting Srinivasa Rao Mandadapu (2021-09-06 06:27:34) > > > +#define LPASS_CDC_DMA_VA0 8 > > > +#define LPASS_MAX_PORTS 9 > > Do we need LPASS_MAX_PORTS in the binding? > Yes. based on this creating array of streams in machine driver. So to make > upper limit introduced this macro. That's saying it's useful to have it in the code, do we need it in the binding itself though?
Thanks for your time and valuable suggestions Srini!! On 9/8/2021 3:11 PM, Srinivas Kandagatla wrote: > > > On 06/09/2021 14:27, Srinivasa Rao Mandadapu wrote: >> +#define LPASS_CDC_DMA_RX0 6 > > You are only adding RX0, what happens to RX1.. RX7 > > TBH, skipping other entries for ex CDC_DMA_RX will end up with sparse > numbering. > > Please add all the entries for CDC_DMA_RX and other ports as well. > > Like: > > #define LPASS_CDC_DMA_RX0 6 > #define LPASS_CDC_DMA_RX1 7 > #define LPASS_CDC_DMA_RX2 8 > #define LPASS_CDC_DMA_RX3 9 > #define LPASS_CDC_DMA_RX4 10 > #define LPASS_CDC_DMA_RX5 11 > #define LPASS_CDC_DMA_RX6 12 > #define LPASS_CDC_DMA_RX7 13 > > Okay. Will add and Share new patch. >> +#define LPASS_CDC_DMA_TX3 7 >> +#define LPASS_CDC_DMA_VA0 8 > You mean VA_TX0? > Yes. Will change accordingly. > >> +#define LPASS_MAX_PORTS 9 > We really do not need this in bindings. > You could add this is some of the driver header files instead. > Okay Will remove. > --srini >> +
Thanks for Your Time Mark Brown!!! On 9/8/2021 3:53 PM, Mark Brown wrote: > On Wed, Sep 08, 2021 at 10:08:33AM +0530, Srinivasa Rao Mandadapu wrote: >> On 9/8/2021 1:54 AM, Stephen Boyd wrote: >>> Quoting Srinivasa Rao Mandadapu (2021-09-06 06:27:34) >>>> +#define LPASS_CDC_DMA_VA0 8 >>>> +#define LPASS_MAX_PORTS 9 >>> Do we need LPASS_MAX_PORTS in the binding? >> Yes. based on this creating array of streams in machine driver. So to make >> upper limit introduced this macro. > That's saying it's useful to have it in the code, do we need it in the > binding itself though? Okay. Got it. will remove it and share new patch.
On Mon, 6 Sep 2021 18:57:34 +0530, Srinivasa Rao Mandadapu wrote: > Add header defining for lpass internal digital codecs rx,tx and va > dai node id's. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: dt-bindings: lpass: add binding headers for digital codecs commit: bfad37c53ae6168d03ab06868ea44e77995c43d5 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/include/dt-bindings/sound/qcom,lpass.h b/include/dt-bindings/sound/qcom,lpass.h index 7b0b80b..187af45 100644 --- a/include/dt-bindings/sound/qcom,lpass.h +++ b/include/dt-bindings/sound/qcom,lpass.h @@ -10,6 +10,11 @@ #define LPASS_DP_RX 5 +#define LPASS_CDC_DMA_RX0 6 +#define LPASS_CDC_DMA_TX3 7 +#define LPASS_CDC_DMA_VA0 8 +#define LPASS_MAX_PORTS 9 + #define LPASS_MCLK0 0 #endif /* __DT_QCOM_LPASS_H */
Add header defining for lpass internal digital codecs rx,tx and va dai node id's. Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org> --- include/dt-bindings/sound/qcom,lpass.h | 5 +++++ 1 file changed, 5 insertions(+)