| Message ID | 20240123111411.850-4-shenghao-ding@ti.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | None | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 23/01/2024 12:14, Shenghao Ding wrote: > PCM6240 driver implements a flexible and configurable setting for register > and filter coefficients, to one, two or even multiple PCM6240 Family Audio > chips. > > Signed-off-by: Shenghao Ding <shenghao-ding@ti.com> Subject: ASoC Subject: rewrite to match something similar to other commits. > > --- > Change in v1: > - Create yaml file for pcm6240 codec driver I don't understand. v1 is the first version. Against what is this change? > --- > .../devicetree/bindings/sound/ti,pcm6240.yaml | 303 ++++++++++++++++++ > 1 file changed, 303 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/ti,pcm6240.yaml > > diff --git a/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml > new file mode 100644 > index 000000000000..59fd48aa4445 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml > @@ -0,0 +1,303 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (C) 2022 - 2024 Texas Instruments Incorporated > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/ti,pcm6240.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments PCM6240 Family Audio ADC/DAC/Router > + > +maintainers: > + - Shenghao Ding <shenghao-ding@ti.com> > + > +description: | > + The PCM6240 Family driver offer a flexible architecture to set the device Describe hardware, not Linux driver. > + number, registers and params for different filters in a bin file. I don't understand entire sentence in context of hardware. > + > + Specifications about the audio chip can be found at: > + https://www.ti.com/lit/gpn/tlv320adc3120 > + https://www.ti.com/lit/gpn/tlv320adc5120 > + https://www.ti.com/lit/gpn/tlv320adc6120 > + https://www.ti.com/lit/gpn/dix4192 > + https://www.ti.com/lit/gpn/pcm1690 > + https://www.ti.com/lit/gpn/pcm3120-q1 > + https://www.ti.com/lit/gpn/pcm3140-q1 > + https://www.ti.com/lit/gpn/pcm5120-q1 > + https://www.ti.com/lit/gpn/pcm6120-q1 > + https://www.ti.com/lit/gpn/pcm6260-q1 > + https://www.ti.com/lit/gpn/pcm9211 > + https://www.ti.com/lit/gpn/pcmd3140 > + https://www.ti.com/lit/gpn/pcmd3180 > + https://www.ti.com/lit/gpn/taa5212 > + https://www.ti.com/lit/gpn/tad5212 > + > +properties: > + compatible: > + description: | > + ti,adc3120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to- > + digital converter (ADC) with 106-dB SNR. > + > + ti,adc5120: 2-Channel, 768-kHz, Burr-BrownTM Audio ADC with 120-dB SNR. > + > + ti,adc6120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to- > + digital converter (ADC) with 123-dB SNR. > + > + ti,pcm1690: 113dB SNR, 24-Bit, 192-kHz Sampling, Enhanced Multi-Level > + ?S, Eight-Channel Audio Digital-to-Analog Converter with Differential > + Outputs. > + > + ti,pcm3120: Automotive, stereo, 106-dB SNR, 768-kHz, low-power > + software-controlled audio ADC. > + > + ti,pcm3140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM Audio ADC > + with 106-dB SNR. > + > + ti,pcm5120: Automotive, stereo, 120-dB SNR, 768-kHz, low-power > + software-controlled audio ADC. > + > + ti,pcm5140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM Audio ADC > + with 120-dB SNR. > + > + ti,pcm6120: Automotive, stereo, 123-dB SNR, 768-kHz, low-power > + software-controlled audio ADC. > + > + ti,pcm6140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM Audio ADC > + with 123-dB SNR. > + > + ti,pcm6240: Automotive 4-ch audio ADC with integrated programmable mic > + bias, boost and input diagnostics. > + > + ti,pcm6260: Automotive 6-ch audio ADC with integrated programmable mic > + bias, boost and input diagnostics. > + > + ti,pcm9211: 216-kHz Digital Audio Interface Transceiver (DIX) > + With Stereo ADC and Routing. > + > + ti,pcmd3140: Four-channel PDM-input to TDM or I�S output converter. > + > + ti,pcmd3180: Eight-channel pulse-density-modulation input to TDM or > + I�S output converter. > + > + ti,taa5212: Low-power high-performance stereo audio ADC with 118-dB > + dynamic range. > + > + ti,tad5212: Low-power stereo audio DAC with 120-dB dynamic range. > + enum: > + - ti,adc3120 > + - ti,adc5120 > + - ti,adc6120 > + - ti,dix4192 > + - ti,pcm1690 > + - ti,pcm3120 > + - ti,pcm3140 > + - ti,pcm5120 > + - ti,pcm5140 > + - ti,pcm6120 > + - ti,pcm6140 > + - ti,pcm6240 > + - ti,pcm6260 > + - ti,pcm9211 > + - ti,pcmd3140 > + - ti,pcmd3180 > + - ti,pcmd512x > + - ti,taa5212 > + - ti,taa5412 > + - ti,tad5212 > + - ti,tad5412 And none of them are compatible with something? > + > + reg: > + description: > + I2C address, in multiple pcmdevices case, all the i2c address > + aggregate as one Audio Device to support multiple audio slots. > + maxItems: 4 > + minItems: 1 minItems, then maxItems. Please open existing bindings and look how it is done there. > + > + reset-gpios: > + maxItems: 1 > + description: > + A GPIO line handling reset of the chip. As the line is active high, > + it should be marked GPIO_ACTIVE_HIGH. Drop description, it's obvious. Or write something useful. > + > + interrupts: > + maxItems: 1 > + description: > + Invalid only for ti,pcm1690 because of no INT pin. > + > + '#sound-dai-cells': > + const: 0 > + > +required: > + - compatible > + - reg > + > +allOf: > + - $ref: dai-common.yaml# > + - if: > + properties: > + compatible: > + contains: > + enum: > + - ti,pcm1690 > + then: > + properties: > + reg: > + description: > + I2C address, in multiple pcmdevices case, all the i2c address > + aggregate as one Audio Device to support multiple audio slots. > + maxItems: 4 > + minItems: 1 > + items: > + minimum: 0x4c > + maximum: 0x4f Why do you repeat the reg constraints? This does not seem needed. > + interrupts: false > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - ti,pcm3140 > + - ti,pcm5140 > + - ti,pcm6140 > + - ti,pcmd3180 > + then: > + properties: > + reg: > + description: > + I2C address, in multiple pcmdevices case, all the i2c address > + aggregate as one Audio Device to support multiple audio slots. > + maxItems: 4 > + minItems: 1 Drop entire if > + items: > + minimum: 0x4c > + maximum: 0x4f > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - ti,adc3120 > + - ti,adc5120 > + - ti,adc6120 > + - ti,pcm3120 > + - ti,pcm5120 > + - ti,pcm6120 > + - ti,pcmd3140 > + then: > + properties: > + reg: > + description: > + I2C address. Just drop description, it is obvious. > + maxItems: 1 > + items: > + maximum: 0x4e > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - ti,dix4192 > + then: > + properties: > + reg: > + description: > + I2C address, in multiple pcmdevices case, all the i2c address > + aggregate as one Audio Device to support multiple audio slots. > + maxItems: 4 > + minItems: 1 > + items: > + minimum: 0x70 > + maximum: 0x73 > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - ti,pcm6240 > + - ti,pcm6260 > + then: > + properties: > + reg: > + description: > + I2C address, in multiple pcmdevices case, all the i2c address > + aggregate as one Audio Device to support multiple audio slots. > + maxItems: 4 > + minItems: 1 > + items: > + minimum: 0x48 > + maximum: 0x4b > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - ti,pcm9211 > + then: > + properties: > + reg: > + description: > + I2C address, in multiple pcmdevices case, all the i2c address > + aggregate as one Audio Device to support multiple audio slots. > + maxItems: 4 > + minItems: 1 > + items: > + minimum: 0x40 > + maximum: 0x43 > + > + - if: > + properties: > + compatible: > + contains: > + enum: > + - ti,taa5212 > + - ti,taa5412 > + - ti,tad5212 > + - ti,tad5412 > + then: > + properties: > + reg: > + description: > + I2C address, in multiple pcmdevices case, all the i2c address > + aggregate as one Audio Device to support multiple audio slots. > + maxItems: 4 > + minItems: 1 > + items: > + minimum: 0x50 > + maximum: 0x53 > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + i2c { > + /* example for two devices with interrupt support */ > + #address-cells = <1>; > + #size-cells = <0>; > + two: pcmdevice@48 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "ti,pcm6240"; > + reg = <0x48>, /* primary-device */ > + <0x4b>; /* secondary-device */ > + #sound-dai-cells = <0>; > + reset-gpios = < &gpio1 10 GPIO_ACTIVE_HIGH >; Drop redundant spaces. Best regards, Krzysztof
On Tue, Jan 23, 2024 at 12:25:04PM +0100, Krzysztof Kozlowski wrote: > On 23/01/2024 12:14, Shenghao Ding wrote: > > --- > > Change in v1: > > - Create yaml file for pcm6240 codec driver > I don't understand. v1 is the first version. Against what is this change? This appears to be a perfectly clear description of the contents of the first version, it's a change against the tree before the patch is applied. It's a bit unusual to include a per version changelog on the first version but not a problem. > > + enum: > > + - ti,adc3120 > > + - ti,adc5120 > > + - ti,adc6120 > > + - ti,dix4192 > > + - ti,pcm1690 > > + - ti,pcm3120 > > + - ti,pcm3140 > > + - ti,pcm5120 > > + - ti,pcm5140 > > + - ti,pcm6120 > > + - ti,pcm6140 > > + - ti,pcm6240 > > + - ti,pcm6260 > > + - ti,pcm9211 > > + - ti,pcmd3140 > > + - ti,pcmd3180 > > + - ti,pcmd512x > > + - ti,taa5212 > > + - ti,taa5412 > > + - ti,tad5212 > > + - ti,tad5412 > And none of them are compatible with something? No idea about these specific chips but that would be entirely normal for CODECs, even where things are subsets there's often some tweaks needed to initialisation or whatever. > > + two: pcmdevice@48 { > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Please be more specific about what you're looking to see there. pcmdevice doesn't seem particularly more specific than something like dsp, it certainly seems within what the text describes.
On 23/01/2024 16:01, Mark Brown wrote: > On Tue, Jan 23, 2024 at 12:25:04PM +0100, Krzysztof Kozlowski wrote: >> On 23/01/2024 12:14, Shenghao Ding wrote: > >>> --- >>> Change in v1: >>> - Create yaml file for pcm6240 codec driver > >> I don't understand. v1 is the first version. Against what is this change? > > This appears to be a perfectly clear description of the contents of the > first version, it's a change against the tree before the patch is > applied. It's a bit unusual to include a per version changelog on the > first version but not a problem. > >>> + enum: >>> + - ti,adc3120 >>> + - ti,adc5120 >>> + - ti,adc6120 >>> + - ti,dix4192 >>> + - ti,pcm1690 >>> + - ti,pcm3120 >>> + - ti,pcm3140 >>> + - ti,pcm5120 >>> + - ti,pcm5140 >>> + - ti,pcm6120 >>> + - ti,pcm6140 >>> + - ti,pcm6240 >>> + - ti,pcm6260 >>> + - ti,pcm9211 >>> + - ti,pcmd3140 >>> + - ti,pcmd3180 >>> + - ti,pcmd512x >>> + - ti,taa5212 >>> + - ti,taa5412 >>> + - ti,tad5212 >>> + - ti,tad5412 > >> And none of them are compatible with something? > > No idea about these specific chips but that would be entirely normal for > CODECs, even where things are subsets there's often some tweaks needed > to initialisation or whatever. I want to double check with the author. > >>> + two: pcmdevice@48 { > >> Node names should be generic. See also an explanation and list of >> examples (not exhaustive) in DT specification: >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > > Please be more specific about what you're looking to see there. > pcmdevice doesn't seem particularly more specific than something like > dsp, it certainly seems within what the text describes. pcm, codec, audio-codec "device" seems redundant, because almost everything is some sort of device. Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Tuesday, January 23, 2024 11:04 PM > To: Mark Brown <broonie@kernel.org> > Cc: Ding, Shenghao <shenghao-ding@ti.com>; conor+dt@kernel.org; > robh+dt@kernel.org; andriy.shevchenko@linux.intel.com; Lu, Kevin <kevin- > lu@ti.com>; Xu, Baojun <baojun.xu@ti.com>; devicetree@vger.kernel.org; > lgirdwood@gmail.com; perex@perex.cz; pierre- > louis.bossart@linux.intel.com; 13916275206@139.com; linux- > sound@vger.kernel.org; linux-kernel@vger.kernel.org; > liam.r.girdwood@intel.com; soyer@irl.hu; Huang, Jonathan > <jkhuang3@ti.com>; tiwai@suse.de; Djuandi, Peter <pdjuandi@ti.com>; > McPherson, Jeff <j-mcpherson@ti.com>; Navada Kanyana, Mukund > <navada@ti.com> > Subject: [EXTERNAL] Re: [PATCH v1 4/4] ASoc: dt-bindings: Create yaml file > for pcm6240 codec driver > > On 23/01/2024 16: 01, Mark Brown wrote: > On Tue, Jan 23, 2024 at > 12: 25: 04PM +0100, Krzysztof Kozlowski wrote: >> On 23/01/2024 12: 14, > Shenghao Ding wrote: > >>> --- >>> Change in v1: >>> - Create yaml > ZjQcmQRYFpfptBannerStart This Message Is From an External Sender This > message came from outside your organization. > > ZjQcmQRYFpfptBannerEnd > On 23/01/2024 16:01, Mark Brown wrote: > > On Tue, Jan 23, 2024 at 12:25:04PM +0100, Krzysztof Kozlowski wrote: > >> On 23/01/2024 12:14, Shenghao Ding wrote: > > > >>> --- > >>> Change in v1: > >>> - Create yaml file for pcm6240 codec driver > > > >> I don't understand. v1 is the first version. Against what is this change? > > > > This appears to be a perfectly clear description of the contents of > > the first version, it's a change against the tree before the patch is > > applied. It's a bit unusual to include a per version changelog on the > > first version but not a problem. > > > >>> + enum: > >>> + - ti,adc3120 > >>> + - ti,adc5120 > >>> + - ti,adc6120 > >>> + - ti,dix4192 > >>> + - ti,pcm1690 > >>> + - ti,pcm3120 > >>> + - ti,pcm3140 > >>> + - ti,pcm5120 > >>> + - ti,pcm5140 > >>> + - ti,pcm6120 > >>> + - ti,pcm6140 > >>> + - ti,pcm6240 > >>> + - ti,pcm6260 > >>> + - ti,pcm9211 > >>> + - ti,pcmd3140 > >>> + - ti,pcmd3180 > >>> + - ti,pcmd512x > >>> + - ti,taa5212 > >>> + - ti,taa5412 > >>> + - ti,tad5212 > >>> + - ti,tad5412 > > > >> And none of them are compatible with something? > > > > No idea about these specific chips but that would be entirely normal > > for CODECs, even where things are subsets there's often some tweaks > > needed to initialisation or whatever. > > I want to double check with the author. > All these chips have only a small feature of codec, such as ADC or DAC, but their audio performance is far superior to the codec's, and cost is lower than codec, and easier to program than codec. Simply one or two register settings can enable them to work. Init for these chips are hardware reset or software reset. As to some audio filter params for internal filters, it is up to the special user cases, which can be saved into the bin file. The default value also can work well. > > > >>> + two: pcmdevice@48 { > > > >> Node names should be generic. See also an explanation and list of > >> examples (not exhaustive) in DT specification: > >> https://urldefense.com/v3/__https://devicetree-specification.readthed > >> ocs.io/en/latest/chapter2-devicetree-basics.html*generic-names- > recomm > >> > endation__;Iw!!G3vK!UaR7OlVhIZucD0CMyBe4iiYyWCl4k_ExXJ6HHG3Fz3qy4 > OPWH > >> esNTs-57iZ8MjCUYfaxVLq8OOGOuOYvkNsth-H6y1cO$ > > > > Please be more specific about what you're looking to see there. > > pcmdevice doesn't seem particularly more specific than something like > > dsp, it certainly seems within what the text describes. > > pcm, codec, audio-codec > "device" seems redundant, because almost everything is some sort of device. > > Best regards, > Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Tuesday, January 23, 2024 7:25 PM > To: Ding, Shenghao <shenghao-ding@ti.com>; broonie@kernel.org; > conor+dt@kernel.org > Cc: robh+dt@kernel.org; andriy.shevchenko@linux.intel.com; Lu, Kevin > <kevin-lu@ti.com>; Xu, Baojun <baojun.xu@ti.com>; > devicetree@vger.kernel.org; lgirdwood@gmail.com; perex@perex.cz; > pierre-louis.bossart@linux.intel.com; 13916275206@139.com; linux- > sound@vger.kernel.org; linux-kernel@vger.kernel.org; > liam.r.girdwood@intel.com; soyer@irl.hu; Huang, Jonathan > <jkhuang3@ti.com>; tiwai@suse.de; Djuandi, Peter <pdjuandi@ti.com>; > McPherson, Jeff <j-mcpherson@ti.com>; Navada Kanyana, Mukund > <navada@ti.com> > Subject: [EXTERNAL] Re: [PATCH v1 4/4] ASoc: dt-bindings: Create yaml file > for pcm6240 codec driver > > On 23/01/2024 12: 14, Shenghao Ding wrote: > PCM6240 driver implements > a flexible and configurable setting for register > and filter coefficients, to one, > two or even multiple PCM6240 Family Audio > chips. > > Signed-off-by: > Shenghao ZjQcmQRYFpfptBannerStart This Message Is From an External > Sender This message came from outside your organization. > > ZjQcmQRYFpfptBannerEnd > On 23/01/2024 12:14, Shenghao Ding wrote: > > PCM6240 driver implements a flexible and configurable setting for > > register and filter coefficients, to one, two or even multiple PCM6240 > > Family Audio chips. > > > > > > Signed-off-by: Shenghao Ding <shenghao-ding@ti.com> > > Subject: ASoC > Subject: rewrite to match something similar to other commits. > > > > > --- > > Change in v1: > > - Create yaml file for pcm6240 codec driver > > I don't understand. v1 is the first version. Against what is this change? > > > --- > > .../devicetree/bindings/sound/ti,pcm6240.yaml | 303 > > ++++++++++++++++++ > > 1 file changed, 303 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/sound/ti,pcm6240.yaml > > > > diff --git a/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml > > b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml > > new file mode 100644 > > index 000000000000..59fd48aa4445 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml > > @@ -0,0 +1,303 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) # Copyright > > +(C) 2022 - 2024 Texas Instruments Incorporated %YAML 1.2 > > +--- > > +$id: > > +https://urldefense.com/v3/__http://devicetree.org/schemas/sound/ti,pc > > +m6240.yaml*__;Iw!!G3vK!QZhdCaAh-suxnL97gv8Mo9tJ- > fyaOPhGTFhz_k3OChSZUx > > +mcQn32yvufRdtd0iMVyU68kXV-mSkyrVHRCzlwdqtrvfRV$ > > +$schema: > > +https://urldefense.com/v3/__http://devicetree.org/meta- > schemas/core.y > > +aml*__;Iw!!G3vK!QZhdCaAh-suxnL97gv8Mo9tJ- > fyaOPhGTFhz_k3OChSZUxmcQn32y > > +vufRdtd0iMVyU68kXV-mSkyrVHRCzlwdo9WSaIa$ > > + > > +title: Texas Instruments PCM6240 Family Audio ADC/DAC/Router > > + > > +maintainers: > > + - Shenghao Ding <shenghao-ding@ti.com> > > + > > +description: | > > + The PCM6240 Family driver offer a flexible architecture to set the > > +device > > Describe hardware, not Linux driver. > > > + number, registers and params for different filters in a bin file. > > I don't understand entire sentence in context of hardware. > > > + > > + Specifications about the audio chip can be found at: > > + https://www.ti.com/lit/gpn/tlv320adc3120 > > + https://www.ti.com/lit/gpn/tlv320adc5120 > > + https://www.ti.com/lit/gpn/tlv320adc6120 > > + https://www.ti.com/lit/gpn/dix4192 > > + https://www.ti.com/lit/gpn/pcm1690 > > + https://www.ti.com/lit/gpn/pcm3120-q1 > > + https://www.ti.com/lit/gpn/pcm3140-q1 > > + https://www.ti.com/lit/gpn/pcm5120-q1 > > + https://www.ti.com/lit/gpn/pcm6120-q1 > > + https://www.ti.com/lit/gpn/pcm6260-q1 > > + https://www.ti.com/lit/gpn/pcm9211 > > + https://www.ti.com/lit/gpn/pcmd3140 > > + https://www.ti.com/lit/gpn/pcmd3180 > > + https://www.ti.com/lit/gpn/taa5212 > > + https://www.ti.com/lit/gpn/tad5212 > > + > > +properties: > > + compatible: > > + description: | > > + ti,adc3120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to- > > + digital converter (ADC) with 106-dB SNR. > > + > > + ti,adc5120: 2-Channel, 768-kHz, Burr-BrownTM Audio ADC with 120- > dB SNR. > > + > > + ti,adc6120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to- > > + digital converter (ADC) with 123-dB SNR. > > + > > + ti,pcm1690: 113dB SNR, 24-Bit, 192-kHz Sampling, Enhanced Multi- > Level > > + ?S, Eight-Channel Audio Digital-to-Analog Converter with Differential > > + Outputs. > > + > > + ti,pcm3120: Automotive, stereo, 106-dB SNR, 768-kHz, low-power > > + software-controlled audio ADC. > > + > > + ti,pcm3140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM > Audio ADC > > + with 106-dB SNR. > > + > > + ti,pcm5120: Automotive, stereo, 120-dB SNR, 768-kHz, low-power > > + software-controlled audio ADC. > > + > > + ti,pcm5140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM > Audio ADC > > + with 120-dB SNR. > > + > > + ti,pcm6120: Automotive, stereo, 123-dB SNR, 768-kHz, low-power > > + software-controlled audio ADC. > > + > > + ti,pcm6140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM > Audio ADC > > + with 123-dB SNR. > > + > > + ti,pcm6240: Automotive 4-ch audio ADC with integrated > programmable mic > > + bias, boost and input diagnostics. > > + > > + ti,pcm6260: Automotive 6-ch audio ADC with integrated > programmable mic > > + bias, boost and input diagnostics. > > + > > + ti,pcm9211: 216-kHz Digital Audio Interface Transceiver (DIX) > > + With Stereo ADC and Routing. > > + > > + ti,pcmd3140: Four-channel PDM-input to TDM or I S output converter. > > + > > + ti,pcmd3180: Eight-channel pulse-density-modulation input to TDM or > > + I S output converter. > > + > > + ti,taa5212: Low-power high-performance stereo audio ADC with 118- > dB > > + dynamic range. > > + > > + ti,tad5212: Low-power stereo audio DAC with 120-dB dynamic range. > > + enum: > > + - ti,adc3120 > > + - ti,adc5120 > > + - ti,adc6120 > > + - ti,dix4192 > > + - ti,pcm1690 > > + - ti,pcm3120 > > + - ti,pcm3140 > > + - ti,pcm5120 > > + - ti,pcm5140 > > + - ti,pcm6120 > > + - ti,pcm6140 > > + - ti,pcm6240 > > + - ti,pcm6260 > > + - ti,pcm9211 > > + - ti,pcmd3140 > > + - ti,pcmd3180 > > + - ti,pcmd512x > > + - ti,taa5212 > > + - ti,taa5412 > > + - ti,tad5212 > > + - ti,tad5412 > > And none of them are compatible with something? > > > + > > + reg: > > + description: > > + I2C address, in multiple pcmdevices case, all the i2c address > > + aggregate as one Audio Device to support multiple audio slots. > > + maxItems: 4 > > + minItems: 1 > > minItems, then maxItems. > > Please open existing bindings and look how it is done there. > > > > + > > + reset-gpios: > > + maxItems: 1 > > + description: > > + A GPIO line handling reset of the chip. As the line is active high, > > + it should be marked GPIO_ACTIVE_HIGH. > > Drop description, it's obvious. Or write something useful. > > > + > > + interrupts: > > + maxItems: 1 > > + description: > > + Invalid only for ti,pcm1690 because of no INT pin. > > + > > + '#sound-dai-cells': > > + const: 0 > > + > > +required: > > + - compatible > > + - reg > > + > > +allOf: > > + - $ref: dai-common.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - ti,pcm1690 > > + then: > > + properties: > > + reg: > > + description: > > + I2C address, in multiple pcmdevices case, all the i2c address > > + aggregate as one Audio Device to support multiple audio slots. > > + maxItems: 4 > > + minItems: 1 > > + items: > > + minimum: 0x4c > > + maximum: 0x4f > > Why do you repeat the reg constraints? This does not seem needed. > > > + interrupts: false > > + > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - ti,pcm3140 > > + - ti,pcm5140 > > + - ti,pcm6140 > > + - ti,pcmd3180 > > + then: > > + properties: > > + reg: > > + description: > > + I2C address, in multiple pcmdevices case, all the i2c address > > + aggregate as one Audio Device to support multiple audio slots. > > + maxItems: 4 > > + minItems: 1 > > Drop entire if How to convey ti,pcm1690 does not support interrupt, and others support if I remove this if > > > + items: > > + minimum: 0x4c > > + maximum: 0x4f > > + BR Shenghao Ding
On 25/01/2024 08:39, Ding, Shenghao wrote: >> >> Why do you repeat the reg constraints? This does not seem needed. >> >>> + interrupts: false >>> + >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - ti,pcm3140 >>> + - ti,pcm5140 >>> + - ti,pcm6140 >>> + - ti,pcmd3180 >>> + then: >>> + properties: >>> + reg: >>> + description: >>> + I2C address, in multiple pcmdevices case, all the i2c address >>> + aggregate as one Audio Device to support multiple audio slots. >>> + maxItems: 4 >>> + minItems: 1 >> You did not respond to any of other comments, therefore I assume you agree with them 100% and you will implement them fully. >> Drop entire if > How to convey ti,pcm1690 does not support interrupt, and others support if I remove this if How? There is no pcm1690 here. BTW, add missing line breaks, especially before next blocks like additionalProperties. >> >>> + items: >>> + minimum: 0x4c >>> + maximum: 0x4f >>> + > > BR > Shenghao Ding > Best regards, Krzysztof
> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Sent: Thursday, January 25, 2024 3:50 PM > To: Ding, Shenghao <shenghao-ding@ti.com>; broonie@kernel.org; > conor+dt@kernel.org > Cc: robh+dt@kernel.org; andriy.shevchenko@linux.intel.com; Lu, Kevin > <kevin-lu@ti.com>; Xu, Baojun <baojun.xu@ti.com>; > devicetree@vger.kernel.org; lgirdwood@gmail.com; perex@perex.cz; > pierre-louis.bossart@linux.intel.com; 13916275206@139.com; linux- > sound@vger.kernel.org; linux-kernel@vger.kernel.org; > liam.r.girdwood@intel.com; soyer@irl.hu; Huang, Jonathan > <jkhuang3@ti.com>; tiwai@suse.de; Djuandi, Peter <pdjuandi@ti.com>; > McPherson, Jeff <j-mcpherson@ti.com>; Navada Kanyana, Mukund > <navada@ti.com> > Subject: Re: [EXTERNAL] Re: [PATCH v1 4/4] ASoc: dt-bindings: Create yaml > file for pcm6240 codec driver > > On 25/01/2024 08: 39, Ding, Shenghao wrote: >> >> Why do you repeat the > reg constraints? This does not seem needed. >> >>> + interrupts: false >>> > + >>> + - if: >>> + properties: >>> ZjQcmQRYFpfptBannerStart This message > was sent from outside of Texas Instruments. > Do not click links or open attachments unless you recognize the source of > this email and know the content is safe. > > ZjQcmQRYFpfptBannerEnd > On 25/01/2024 08:39, Ding, Shenghao wrote: > >> > >> Why do you repeat the reg constraints? This does not seem needed. > >> > >>> + interrupts: false > >>> + > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + contains: > >>> + enum: > >>> + - ti,pcm3140 > >>> + - ti,pcm5140 > >>> + - ti,pcm6140 > >>> + - ti,pcmd3180 > >>> + then: > >>> + properties: > >>> + reg: > >>> + description: > >>> + I2C address, in multiple pcmdevices case, all the i2c address > >>> + aggregate as one Audio Device to support multiple audio slots. > >>> + maxItems: 4 > >>> + minItems: 1 > >> > > You did not respond to any of other comments, therefore I assume you > agree with them 100% and you will implement them fully. > > > >> Drop entire if > > How to convey ti,pcm1690 does not support interrupt, and others > > support if I remove this if > > How? There is no pcm1690 here. How can others know that pcm3140, pcm5140, pcm6140 and pcmd3180 Support i2c address from 0x4c to 0x4f, if this if branch was removed. > > BTW, add missing line breaks, especially before next blocks like > additionalProperties. Accepted > > >> > >>> + items: > >>> + minimum: 0x4c > >>> + maximum: 0x4f > >>> + > > > > BR > > Shenghao Ding > > > > Best regards, > Krzysztof
On 25/01/2024 09:46, Ding, Shenghao wrote: > > >> -----Original Message----- >> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Sent: Thursday, January 25, 2024 3:50 PM >> To: Ding, Shenghao <shenghao-ding@ti.com>; broonie@kernel.org; >> conor+dt@kernel.org >> Cc: robh+dt@kernel.org; andriy.shevchenko@linux.intel.com; Lu, Kevin >> <kevin-lu@ti.com>; Xu, Baojun <baojun.xu@ti.com>; >> devicetree@vger.kernel.org; lgirdwood@gmail.com; perex@perex.cz; >> pierre-louis.bossart@linux.intel.com; 13916275206@139.com; linux- >> sound@vger.kernel.org; linux-kernel@vger.kernel.org; >> liam.r.girdwood@intel.com; soyer@irl.hu; Huang, Jonathan >> <jkhuang3@ti.com>; tiwai@suse.de; Djuandi, Peter <pdjuandi@ti.com>; >> McPherson, Jeff <j-mcpherson@ti.com>; Navada Kanyana, Mukund >> <navada@ti.com> >> Subject: Re: [EXTERNAL] Re: [PATCH v1 4/4] ASoc: dt-bindings: Create yaml >> file for pcm6240 codec driver >> >> On 25/01/2024 08: 39, Ding, Shenghao wrote: >> >> Why do you repeat the >> reg constraints? This does not seem needed. >> >>> + interrupts: false >>> >> + >>> + - if: >>> + properties: >>> ZjQcmQRYFpfptBannerStart This message >> was sent from outside of Texas Instruments. >> Do not click links or open attachments unless you recognize the source of >> this email and know the content is safe. >> >> ZjQcmQRYFpfptBannerEnd >> On 25/01/2024 08:39, Ding, Shenghao wrote: >>>> >>>> Why do you repeat the reg constraints? This does not seem needed. >>>> >>>>> + interrupts: false >>>>> + >>>>> + - if: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - ti,pcm3140 >>>>> + - ti,pcm5140 >>>>> + - ti,pcm6140 >>>>> + - ti,pcmd3180 >>>>> + then: >>>>> + properties: >>>>> + reg: >>>>> + description: >>>>> + I2C address, in multiple pcmdevices case, all the i2c address >>>>> + aggregate as one Audio Device to support multiple audio slots. >>>>> + maxItems: 4 >>>>> + minItems: 1 >>>> >> >> You did not respond to any of other comments, therefore I assume you >> agree with them 100% and you will implement them fully. >> >> >>>> Drop entire if >>> How to convey ti,pcm1690 does not support interrupt, and others >>> support if I remove this if >> >> How? There is no pcm1690 here. > How can others know that pcm3140, pcm5140, pcm6140 and pcmd3180 > Support i2c address from 0x4c to 0x4f, if this if branch was removed. They do not need anything to support any specific I2C address... If ever wondering, open some recent bindings and take a look how it is done there. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml new file mode 100644 index 000000000000..59fd48aa4445 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml @@ -0,0 +1,303 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (C) 2022 - 2024 Texas Instruments Incorporated +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/ti,pcm6240.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments PCM6240 Family Audio ADC/DAC/Router + +maintainers: + - Shenghao Ding <shenghao-ding@ti.com> + +description: | + The PCM6240 Family driver offer a flexible architecture to set the device + number, registers and params for different filters in a bin file. + + Specifications about the audio chip can be found at: + https://www.ti.com/lit/gpn/tlv320adc3120 + https://www.ti.com/lit/gpn/tlv320adc5120 + https://www.ti.com/lit/gpn/tlv320adc6120 + https://www.ti.com/lit/gpn/dix4192 + https://www.ti.com/lit/gpn/pcm1690 + https://www.ti.com/lit/gpn/pcm3120-q1 + https://www.ti.com/lit/gpn/pcm3140-q1 + https://www.ti.com/lit/gpn/pcm5120-q1 + https://www.ti.com/lit/gpn/pcm6120-q1 + https://www.ti.com/lit/gpn/pcm6260-q1 + https://www.ti.com/lit/gpn/pcm9211 + https://www.ti.com/lit/gpn/pcmd3140 + https://www.ti.com/lit/gpn/pcmd3180 + https://www.ti.com/lit/gpn/taa5212 + https://www.ti.com/lit/gpn/tad5212 + +properties: + compatible: + description: | + ti,adc3120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to- + digital converter (ADC) with 106-dB SNR. + + ti,adc5120: 2-Channel, 768-kHz, Burr-BrownTM Audio ADC with 120-dB SNR. + + ti,adc6120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to- + digital converter (ADC) with 123-dB SNR. + + ti,pcm1690: 113dB SNR, 24-Bit, 192-kHz Sampling, Enhanced Multi-Level + ?S, Eight-Channel Audio Digital-to-Analog Converter with Differential + Outputs. + + ti,pcm3120: Automotive, stereo, 106-dB SNR, 768-kHz, low-power + software-controlled audio ADC. + + ti,pcm3140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM Audio ADC + with 106-dB SNR. + + ti,pcm5120: Automotive, stereo, 120-dB SNR, 768-kHz, low-power + software-controlled audio ADC. + + ti,pcm5140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM Audio ADC + with 120-dB SNR. + + ti,pcm6120: Automotive, stereo, 123-dB SNR, 768-kHz, low-power + software-controlled audio ADC. + + ti,pcm6140: Automotive, Quad-Channel, 768-kHz, Burr-BrownTM Audio ADC + with 123-dB SNR. + + ti,pcm6240: Automotive 4-ch audio ADC with integrated programmable mic + bias, boost and input diagnostics. + + ti,pcm6260: Automotive 6-ch audio ADC with integrated programmable mic + bias, boost and input diagnostics. + + ti,pcm9211: 216-kHz Digital Audio Interface Transceiver (DIX) + With Stereo ADC and Routing. + + ti,pcmd3140: Four-channel PDM-input to TDM or I�S output converter. + + ti,pcmd3180: Eight-channel pulse-density-modulation input to TDM or + I�S output converter. + + ti,taa5212: Low-power high-performance stereo audio ADC with 118-dB + dynamic range. + + ti,tad5212: Low-power stereo audio DAC with 120-dB dynamic range. + enum: + - ti,adc3120 + - ti,adc5120 + - ti,adc6120 + - ti,dix4192 + - ti,pcm1690 + - ti,pcm3120 + - ti,pcm3140 + - ti,pcm5120 + - ti,pcm5140 + - ti,pcm6120 + - ti,pcm6140 + - ti,pcm6240 + - ti,pcm6260 + - ti,pcm9211 + - ti,pcmd3140 + - ti,pcmd3180 + - ti,pcmd512x + - ti,taa5212 + - ti,taa5412 + - ti,tad5212 + - ti,tad5412 + + reg: + description: + I2C address, in multiple pcmdevices case, all the i2c address + aggregate as one Audio Device to support multiple audio slots. + maxItems: 4 + minItems: 1 + + reset-gpios: + maxItems: 1 + description: + A GPIO line handling reset of the chip. As the line is active high, + it should be marked GPIO_ACTIVE_HIGH. + + interrupts: + maxItems: 1 + description: + Invalid only for ti,pcm1690 because of no INT pin. + + '#sound-dai-cells': + const: 0 + +required: + - compatible + - reg + +allOf: + - $ref: dai-common.yaml# + - if: + properties: + compatible: + contains: + enum: + - ti,pcm1690 + then: + properties: + reg: + description: + I2C address, in multiple pcmdevices case, all the i2c address + aggregate as one Audio Device to support multiple audio slots. + maxItems: 4 + minItems: 1 + items: + minimum: 0x4c + maximum: 0x4f + interrupts: false + + - if: + properties: + compatible: + contains: + enum: + - ti,pcm3140 + - ti,pcm5140 + - ti,pcm6140 + - ti,pcmd3180 + then: + properties: + reg: + description: + I2C address, in multiple pcmdevices case, all the i2c address + aggregate as one Audio Device to support multiple audio slots. + maxItems: 4 + minItems: 1 + items: + minimum: 0x4c + maximum: 0x4f + + - if: + properties: + compatible: + contains: + enum: + - ti,adc3120 + - ti,adc5120 + - ti,adc6120 + - ti,pcm3120 + - ti,pcm5120 + - ti,pcm6120 + - ti,pcmd3140 + then: + properties: + reg: + description: + I2C address. + maxItems: 1 + items: + maximum: 0x4e + + - if: + properties: + compatible: + contains: + enum: + - ti,dix4192 + then: + properties: + reg: + description: + I2C address, in multiple pcmdevices case, all the i2c address + aggregate as one Audio Device to support multiple audio slots. + maxItems: 4 + minItems: 1 + items: + minimum: 0x70 + maximum: 0x73 + + - if: + properties: + compatible: + contains: + enum: + - ti,pcm6240 + - ti,pcm6260 + then: + properties: + reg: + description: + I2C address, in multiple pcmdevices case, all the i2c address + aggregate as one Audio Device to support multiple audio slots. + maxItems: 4 + minItems: 1 + items: + minimum: 0x48 + maximum: 0x4b + + - if: + properties: + compatible: + contains: + enum: + - ti,pcm9211 + then: + properties: + reg: + description: + I2C address, in multiple pcmdevices case, all the i2c address + aggregate as one Audio Device to support multiple audio slots. + maxItems: 4 + minItems: 1 + items: + minimum: 0x40 + maximum: 0x43 + + - if: + properties: + compatible: + contains: + enum: + - ti,taa5212 + - ti,taa5412 + - ti,tad5212 + - ti,tad5412 + then: + properties: + reg: + description: + I2C address, in multiple pcmdevices case, all the i2c address + aggregate as one Audio Device to support multiple audio slots. + maxItems: 4 + minItems: 1 + items: + minimum: 0x50 + maximum: 0x53 +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + i2c { + /* example for two devices with interrupt support */ + #address-cells = <1>; + #size-cells = <0>; + two: pcmdevice@48 { + compatible = "ti,pcm6240"; + reg = <0x48>, /* primary-device */ + <0x4b>; /* secondary-device */ + #sound-dai-cells = <0>; + reset-gpios = < &gpio1 10 GPIO_ACTIVE_HIGH >; + interrupt-parent = <&gpio1>; + interrupts = <15>; + }; + }; + - | + #include <dt-bindings/gpio/gpio.h> + i2c { + /* example for one device without interrupt support*/ + #address-cells = <1>; + #size-cells = <0>; + one: pcmdevice@4c { + compatible = "ti,pcmd3180"; + reg = <0x4c>; + #sound-dai-cells = <0>; + reset-gpios = < &gpio1 10 GPIO_ACTIVE_HIGH >; + }; + }; +...
PCM6240 driver implements a flexible and configurable setting for register and filter coefficients, to one, two or even multiple PCM6240 Family Audio chips. Signed-off-by: Shenghao Ding <shenghao-ding@ti.com> --- Change in v1: - Create yaml file for pcm6240 codec driver --- .../devicetree/bindings/sound/ti,pcm6240.yaml | 303 ++++++++++++++++++ 1 file changed, 303 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/ti,pcm6240.yaml