Received-SPF: pass (google.com: domain of linux-kernel+bounces-144534-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <85ea2b33-05ac-400f-8017-5539d21ebe32@ti.com>
Date: Mon, 15 Apr 2024 10:42:45 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] arm64: dts: ti: k3-j784s4-evm: Add support for multiple
 CAN instances
To: Bhavya Kapoor <b-kapoor@ti.com>, <linux-kernel@vger.kernel.org>,
        <devicetree@vger.kernel.org>, <vigneshr@ti.com>
CC: <linux-arm-kernel@lists.infradead.org>, <conor+dt@kernel.org>,
        <krzk+dt@kernel.org>, <robh@kernel.org>, <kristo@kernel.org>,
        <nm@ti.com>, <u-kumar1@ti.com>
References: <20240411201747.18697-1-b-kapoor@ti.com>
Content-Language: en-US
From: "Kumar, Udit" <u-kumar1@ti.com>
In-Reply-To: <20240411201747.18697-1-b-kapoor@ti.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit

Hello Bhavya,

On 4/12/2024 1:47 AM, Bhavya Kapoor wrote:
> CAN instances 0 and 1 in the mcu domain and 16 in the main domain are
> brought on the evm through headers J42, J43 and J46 respectively. Thus,
> add their respective transceiver's 0, 1 and 2 dt nodes to add support
> for these CAN instances.

Looking at schematic and board data sheet, it appears, board has 6 CAN 
interfaces.

[J41--J46],  but we are enabling only 4.

I understand, other interfaces might be used for other purpose.

Vignesh/Nishanth,

Do you think, this make sense to code all available interfaces on board and

mark as reserved, if used for other purpose , similar to what is done 
for wkup_uart for this board.


> CAN instance 4 in the main domain is brought on the evm through header
> J45. The CAN High and Low lines from the SoC are routed through a mux
> on the evm. The select lines need to be set for the CAN signals to
> reach to its transceiver on the evm. Therefore, add transceiver 3
> dt node to add support for this CAN instance.
>
> Signed-off-by: Bhavya Kapoor <b-kapoor@ti.com>
> ---
>
> rebased to next-20240411
>
>   arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 107 +++++++++++++++++++++++
>   1 file changed, 107 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> index 81fd7afac8c5..e56901973895 100644
> --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
> @@ -272,6 +272,45 @@ dp0_connector_in: endpoint {
>   			};
>   		};
>   	};
> +
> +	transceiver0: can-phy0 {
> +		compatible = "ti,tcan1042";
> +		#phy-cells = <0>;
> +		max-bitrate = <5000000>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&mcu_mcan0_gpio_pins_default>;
> +		standby-gpios = <&wkup_gpio0 69 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	transceiver1: can-phy1 {
> +		compatible = "ti,tcan1042";
> +		#phy-cells = <0>;
> +		max-bitrate = <5000000>;
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&mcu_mcan1_gpio_pins_default>;
> +		standby-gpios = <&wkup_gpio0 2 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	transceiver2: can-phy2 {
> +		/* standby pin has been grounded by default */
> +		compatible = "ti,tcan1042";
> +		#phy-cells = <0>;
> +		max-bitrate = <5000000>;
> +	};
> +
> +	transceiver3: can-phy3 {
> +		compatible = "ti,tcan1042";
> +		#phy-cells = <0>;
> +		max-bitrate = <5000000>;
> +		standby-gpios = <&exp2 7 GPIO_ACTIVE_HIGH>;
> +		mux-states = <&mux1 1>;
> +	};
> +
> +	mux1: mux-controller {
> +		compatible = "gpio-mux";
> +		#mux-state-cells = <1>;
> +		mux-gpios = <&exp2 14 GPIO_ACTIVE_HIGH>;

Could you help here on logic to choose pin 14

As I see for this mux

S0 is CANUART_MUX_SEL0 [Pin 14, which you want to control as high]

S1 is Pin 15 is "CANUART_MUX2_SEL1"

S2 is high

So in order to get CAN on mux output

All, S0, S1 and S2 should be high.

IMO, you should control both S0 and S1, or just S1 and S0 with dip switch

> +	};
>   };
>   
>   &wkup_gpio0 {
> @@ -336,6 +375,20 @@ J784S4_IOPAD(0x014, PIN_INPUT_PULLUP, 8) /* (AG33) MCAN14_TX.I2C4_SCL */
>   			J784S4_IOPAD(0x010, PIN_INPUT_PULLUP, 8) /* (AH33) MCAN13_RX.I2C4_SDA */
>   		>;
>   	};
> +
> +	main_mcan4_pins_default: main-mcan4-default-pins {
> +		pinctrl-single,pins = <
> +			J784S4_IOPAD(0x088, PIN_INPUT, 0) /* (AF36) MCAN4_RX */
> +			J784S4_IOPAD(0x084, PIN_OUTPUT, 0) /* (AG38) MCAN4_TX */
> +		>;
> +	};
> +
> +	main_mcan16_pins_default: main-mcan16-default-pins {
> +		pinctrl-single,pins = <
> +			J784S4_IOPAD(0x028, PIN_INPUT, 0) /* (AE33) MCAN16_RX */
> +			J784S4_IOPAD(0x024, PIN_OUTPUT, 0) /* (AH34) MCAN16_TX */
> +		>;
> +	};
>   };
>   
>   &wkup_pmx2 {
> @@ -415,6 +468,32 @@ J784S4_WKUP_IOPAD(0x104, PIN_INPUT, 0) /* (U33) MCU_ADC1_AIN6 */
>   			J784S4_WKUP_IOPAD(0x108, PIN_INPUT, 0) /* (Y36) MCU_ADC1_AIN7 */
>   		>;
>   	};
> +
> +	mcu_mcan0_pins_default: mcu-mcan0-default-pins {
> +		pinctrl-single,pins = <
> +			J784S4_WKUP_IOPAD(0x050, PIN_OUTPUT, 0) /* (K33) MCU_MCAN0_TX */
> +			J784S4_WKUP_IOPAD(0x054, PIN_INPUT, 0) /* (F38) MCU_MCAN0_RX */
> +		>;
> +	};
> +
> +	mcu_mcan1_pins_default: mcu-mcan1-default-pins {
> +		pinctrl-single,pins = <
> +			J784S4_WKUP_IOPAD(0x068, PIN_OUTPUT, 0) /* (H35) WKUP_GPIO0_4.MCU_MCAN1_TX */
> +			J784S4_WKUP_IOPAD(0x06c, PIN_INPUT, 0) /* (K36) WKUP_GPIO0_5.MCU_MCAN1_RX */
> +		>;
> +	};
> +
> +	mcu_mcan0_gpio_pins_default: mcu-mcan0-gpio-default-pins {
> +		pinctrl-single,pins = <
> +			J784S4_WKUP_IOPAD(0x040, PIN_INPUT, 7) /* (J38) MCU_SPI0_D1.WKUP_GPIO0_69 */
> +		>;
> +	};
> +
> +	mcu_mcan1_gpio_pins_default: mcu-mcan1-gpio-default-pins {
> +		pinctrl-single,pins = <
> +			J784S4_WKUP_IOPAD(0x060, PIN_INPUT, 7) /* (J35) WKUP_GPIO0_2 */
> +		>;
> +	};
>   };
>   
>   &wkup_pmx1 {
> @@ -1105,3 +1184,31 @@ dp0_out: endpoint {
>   		};
>   	};
>   };
> +
> +&mcu_mcan0 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mcu_mcan0_pins_default>;
> +	phys = <&transceiver0>;
> +};
> +
> +&mcu_mcan1 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&mcu_mcan1_pins_default>;
> +	phys = <&transceiver1>;
> +};
> +
> +&main_mcan16 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&main_mcan16_pins_default>;
> +	phys = <&transceiver2>;
> +};
> +
> +&main_mcan4 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&main_mcan4_pins_default>;
> +	phys = <&transceiver3>;
> +};