Re: [PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support

From: "Anwar, Md Danish" <a0501179@ti.com>
To: Nishanth Menon <nm@ti.com>
Cc: MD Danish Anwar <danishanwar@ti.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Conor Dooley <conor+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Rob Herring <robh+dt@kernel.org>, <linux-kernel@vger.kernel.org>,
	<devicetree@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Tero Kristo <kristo@kernel.org>, <srk@ti.com>,
	<r-gunasekaran@ti.com>
Subject: Re: [PATCH 2/3] arm64: dts: ti: k3-am642-evm: add ICSSG1 Ethernet support
Date: Thu, 7 Dec 2023 19:23:59 +0530	[thread overview]
Message-ID: <6f2c1c23-bed6-48be-8407-030f2993ec8a@ti.com> (raw)
In-Reply-To: <20231207134343.ufiy2owik5kn3y2r@degrease>

On 12/7/2023 7:13 PM, Nishanth Menon wrote:
> On 18:58-20231207, Anwar, Md Danish wrote:
> [...]
>>>> +
>>>>  	memory@80000000 {
>>>>  		bootph-all;
>>>>  		device_type = "memory";
>>>> @@ -229,6 +234,70 @@ transceiver2: can-phy1 {
>>>>  		max-bitrate = <5000000>;
>>>>  		standby-gpios = <&exp1 9 GPIO_ACTIVE_HIGH>;
>>>>  	};
>>>> +
>>>> +	icssg1_eth: icssg1-eth {
>>>> +		compatible = "ti,am642-icssg-prueth";
>>>> +		pinctrl-names = "default";
>>>> +		pinctrl-0 = <&icssg1_rgmii1_pins_default>;
>>>> +
>>>> +		sram = <&oc_sram>;
>>>> +		ti,prus = <&pru1_0>, <&rtu1_0>, <&tx_pru1_0>, <&pru1_1>, <&rtu1_1>, <&tx_pru1_1>;
>>>> +		firmware-name = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
>>>> +				"ti-pruss/am65x-sr2-txpru1-prueth-fw.elf";
>>>
>>> Umm... am65x??? is that a typo? I'd rather keep it am64x here and drop
>>> that sr2 thing. Tomorrow there will be a custom bug on am64 and then we
>>> will have to respin this again.
>>>
>>
>> No Nishant, this is not a typo. Both AM64x and AM65x use the same ICSSG
>> firmwares. We only have am65x-sr2-* firmwares and they are used by both
>> AM64x and AM65x and that is why I have kept the firmware-name here in dt
>> same as the files that we load on the pru cores.
>>
> 
> SoCs are different. The hardware as a result is different as well. In
> fact, you do have a different compatible to distinguish the two. Some
> day, there will be an erratum that is different and we will be stuck
> with abi breakage across distros. So, unless you can explain why this
> scenario will never occur, I don't buy the argument this will survive
> long term.
> 

Agreed, this property was introduced for this purpose only. Today am65x
and am64x share the same firmware however in future the firmwares might
change and that is why we have this property. Currently this property is
not used in driver and firmware name is defined in the driver (with the
below structure) which is used for both am64x and am65x. I will rename
the firmware names here to am64x-sr2* in v2. In future when we have
different firmwares for different SoCs, we can stop using the below
structure and use the firmware-name property from dt.

static struct icssg_firmwares icssg_emac_firmwares[] = {
	{
		.pru = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
		.rtu = "ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
		.txpru = "ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
	},
	{
		.pru = "ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
		.rtu = "ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
		.txpru = "ti-pruss/am65x-sr2-txpru1-prueth-fw.elf",
	}
};

-- 
Thanks and Regards,
Md Danish Anwar

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel