[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <9a443016-49a9-4e23-b93c-59192a1b5a4c@ti.com>
Date: Wed, 4 Feb 2026 16:48:03 +0530
From: Moteen Shah <m-shah@...com>
To: Nishanth Menon <nm@...com>
CC: <krzk+dt@...nel.org>, <robh@...nel.org>, <conor+dt@...nel.org>,
<vigneshr@...com>, <kristo@...nel.org>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
<u-kumar1@...com>, <gehariprasath@...com>, <y-abhilashchandra@...com>
Subject: Re: [PATCH 3/3] arm64: dts: ti: k3-j721s2-common-proc-board: Enable
analog audio support
Hey Nishanth,
Thanks for the reviews.
On 16/01/26 18:34, Nishanth Menon wrote:
> On 16:15-20260112, Moteen Shah wrote:
>> From: Jayesh Choudhary <j-choudhary@...com>
>>
>> The audio support on J721S2-EVM is using PCM3168A codec
>> connected to McASP4 serializers.
>>
>> - Add the nodes for sound-card, audio codec, MAIN_I2C3 and
>> McASP4.
>> - Add pinmux for I2C3, McASP4, AUDIO_EXT_REFCLK1 and
>> WKUP_GPIO_0.
>> - Add necessary GPIO hogs to route the MAIN_I2C3 lines and
>> McASP serializer.
>> - Add idle-state as 1 in mux0 and mux1 to route McASP signals
>>
>> Signed-off-by: Jayesh Choudhary <j-choudhary@...com>
>> Signed-off-by: Moteen Shah <m-shah@...com>
>> ---
>> .../dts/ti/k3-j721s2-common-proc-board.dts | 131 ++++++++++++++++++
>> 1 file changed, 131 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
>> index 4fea99519113..d9269a16956c 100644
>> --- a/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-j721s2-common-proc-board.dts
>> @@ -150,6 +150,28 @@ transceiver4: can-phy4 {
>> standby-gpios = <&exp_som 7 GPIO_ACTIVE_HIGH>;
>> mux-states = <&mux1 1>;
>> };
>> +
>> + codec_audio: sound {
>> + compatible = "ti,j7200-cpb-audio";
>> + model = "j721e-cpb";
>> +
>> + ti,cpb-mcasp = <&mcasp4>;
>> + ti,cpb-codec = <&pcm3168a_1>;
>> +
>> + clocks = <&k3_clks 213 0>, <&k3_clks 213 1>,
>> + <&k3_clks 157 299>, <&k3_clks 157 328>;
>> + clock-names = "cpb-mcasp-auxclk", "cpb-mcasp-auxclk-48000",
>> + "cpb-codec-scki", "cpb-codec-scki-48000";
>> + };
>> +
>> + i2c_mux: mux-controller-2 {
> There is just a single i2c_mux on processor board/evm? if not, might
> be good to set this as i2c_mux0 or appropriate naming?
There are others as well, will rename this to i2c_mux0
>
>> + compatible = "gpio-mux";
>> + #mux-state-cells = <1>;
>> + mux-gpios = <&wkup_gpio0 54 GPIO_ACTIVE_HIGH>;
>> + idle-state = <1>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&main_i2c3_mux_pins_default>;
>> + };
>> };
>>
>> &main_pmx0 {
>> @@ -224,6 +246,22 @@ J721S2_IOPAD(0x03c, PIN_INPUT, 0) /* (U27) MCASP0_AFSX.MCAN5_RX */
>> J721S2_IOPAD(0x038, PIN_OUTPUT, 0) /* (AB28) MCASP0_ACLKX.MCAN5_TX */
>> >;
>> };
>> +
>> + mcasp4_pins_default: mcasp4-default-pins {
>> + pinctrl-single,pins = <
>> + J721S2_IOPAD(0x0c8, PIN_OUTPUT_PULLDOWN, 1) /* (AD28) MCASP4_ACLKX */
>> + J721S2_IOPAD(0x06c, PIN_OUTPUT_PULLDOWN, 1) /* (V26) MCASP4_AFSX */
>> + J721S2_IOPAD(0x068, PIN_INPUT_PULLDOWN, 1) /* (U28) MCASP4_AXR1 */
>> + J721S2_IOPAD(0x0c4, PIN_OUTPUT_PULLDOWN, 1) /* (AB26) MCASP4_AXR2 */
>> + J721S2_IOPAD(0x070, PIN_OUTPUT_PULLDOWN, 1) /* (R27) MCASP4_AXR3 */
>> + >;
>> + };
>> +
>> + audio_ext_refclk1_pins_default: audio-ext-refclk1-default-pins {
>> + pinctrl-single,pins = <
>> + J721S2_IOPAD(0x078, PIN_OUTPUT, 1) /* (Y25) MCAN2_RX.AUDIO_EXT_REFCLK1 */
>> + >;
>> + };
>> };
>>
>> &wkup_pmx2 {
>> @@ -321,6 +359,12 @@ J721S2_WKUP_IOPAD(0x104, PIN_INPUT, 0) /* (N26) MCU_ADC1_AIN6 */
>> J721S2_WKUP_IOPAD(0x108, PIN_INPUT, 0) /* (N27) MCU_ADC1_AIN7 */
>> >;
>> };
>> +
>> + main_i2c3_mux_pins_default: main-i2c3-mux-default-pins {
>> + pinctrl-single,pins = <
>> + J721S2_WKUP_IOPAD(0x038, PIN_OUTPUT, 7) /* (B27) WKUP_GPIO0_54 */
>> + >;
>> + };
>> };
>>
>> &wkup_pmx1 {
>> @@ -396,6 +440,22 @@ exp2: gpio@22 {
>> "MLB_MUX_SEL", "MCAN_MUX_SEL", "MCASP2/SPI3_MUX_SEL", "PCIe_CLKREQn_MUX_SEL",
>> "CDCI2_RSTZ", "ENET_EXP_PWRDN", "ENET_EXP_RESETZ", "ENET_I2CMUX_SEL",
>> "ENET_EXP_SPARE2", "M2PCIE_RTSZ", "USER_INPUT1", "USER_LED1", "USER_LED2";
>> +
>> + p09-hog {
>> + /* P09 - MCASP/TRACE_MUX_S0 */
>> + gpio-hog;
>> + gpios = <9 GPIO_ACTIVE_HIGH>;
>> + output-low;
>> + line-name = "MCASP/TRACE_MUX_S0";
>> + };
>> +
>> + p10-hog {
>> + /* P10 - MCASP/TRACE_MUX_S1 */
>> + gpio-hog;
>> + gpios = <10 GPIO_ACTIVE_HIGH>;
>> + output-high;
>> + line-name = "MCASP/TRACE_MUX_S1";
>> + };
> We loose JTAG Trace?
That is true, will pivot to using the dtbo method instead, out of box we
still want the JTAG trace to be active
>
>> };
>> };
>>
>> @@ -657,3 +717,74 @@ &dphy_tx0 {
>> &dsi0 {
>> status = "okay";
>> };
>> +
>> +&mux0 {
>> + idle-state = <0>;
>> +};
>> +
>> +&mux1 {
>> + idle-state = <0>;
> Commit says:
> - Add idle-state as 1 in mux0 and mux1 to route McASP signals
>
> you are adding idle-state = <0> here - which is correct? why?
I will fix the commit msg in the revision, the state should be 0 for the
sound card to get registered.
>
>> +};
>> +
>> +&exp_som {
>> + p03-hog {
>> + /* P03 - CANUART_MUX_SEL1 */
> and we loose CAN? or UART -> not sure I understand this change and why
> is it related to audio?
I checked the schematics[0] of the SoM and seems like it is irrelevant,
I'll drop this in the revision.
>
>> + gpio-hog;
>> + gpios = <3 GPIO_ACTIVE_HIGH>;
>> + output-high;
>> + line-name = "CANUART_MUX_SEL1";
>> + };
>> +};
>> +
>> +&k3_clks {
>> + /* Confiure AUDIO_EXT_REFCLK1 pin as output */
> I bet you did'nt run checkpatch --strict --codespell.. :)
> s/Confiure/Configure
Didn't run it with codespell, will rectify in the revision, apologies.
>
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&audio_ext_refclk1_pins_default>;
>> +};
>> +
>> +&main_i2c3 {
>> + status = "okay";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&main_i2c3_pins_default>;
>> + clock-frequency = <400000>;
>> + mux-states = <&i2c_mux 1>;
>> +
>> + exp3: gpio@20 {
>> + compatible = "ti,tca6408";
>> + reg = <0x20>;
>> + gpio-controller;
>> + #gpio-cells = <2>;
> gpio-line-names please
Noted
[0] https://www.ti.com/lit/zip/SPRR439
Regards,
Moteen
>
>> + };
>> +
>> + pcm3168a_1: audio-codec@44 {
>> + compatible = "ti,pcm3168a";
>> + reg = <0x44>;
>> + #sound-dai-cells = <1>;
>> + reset-gpios = <&exp3 0 GPIO_ACTIVE_LOW>;
>> + /* C_AUDIO_REFCLK1 -> MCAN2_RX (Y25) */
> What is this comment pertinent to?
>
>> + clocks = <&audio_refclk1>;
>> + clock-names = "scki";
>> + VDD1-supply = <&vsys_3v3>;
>> + VDD2-supply = <&vsys_3v3>;
>> + VCCAD1-supply = <&vsys_5v0>;
>> + VCCAD2-supply = <&vsys_5v0>;
>> + VCCDA1-supply = <&vsys_5v0>;
>> + VCCDA2-supply = <&vsys_5v0>;
>> + };
>> +};
>> +
>> +&mcasp4 {
>> + status = "okay";
>> + #sound-dai-cells = <0>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&mcasp4_pins_default>;
>> + op-mode = <0>; /* MCASP_IIS_MODE */
>> + tdm-slots = <2>;
>> + auxclk-fs-ratio = <256>;
>> + serial-dir = < /* 0: INACTIVE, 1: TX, 2: RX */
>> + 0 2 1 1
>> + 0 0 0 0
>> + 0 0 0 0
>> + 0 0 0 0
>> + >;
>> +};
>> --
>> 2.34.1
>>
>>
Powered by blists - more mailing lists