Re: [PATCH v2] arm64: dts: ti: k3-am62p: use eFuse MAC Address for CPSW3G Port 1

[Krzysztof Kozlowski]

On 04/04/2024 10:18, Siddharth Vadapalli wrote:
> Add the "cpsw-mac-efuse" node within "wkup_conf" node corresponding to the
> CTRLMMR_MAC_IDx registers within the CTRL_MMR space. Assign the compatible
> "ti,am62p-cpsw-mac-efuse" to enable "syscon_regmap" operations on these
> registers. The MAC Address programmed in the eFuse is accessible through
> the CTRLMMR_MAC_IDx registers. The "ti,syscon-efuse" device-tree property
> points to the CTRLMMR_MAC_IDx registers, allowing the CPSW driver to fetch
> the MAC Address and assign it to the network interface associated with
> CPSW3G MAC Port 1.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@xxxxxx>
> ---
> 
> This patch is based on linux-next tagged next-20240404.
> Patch depends on:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402105708.4114146-1-s-vadapalli@xxxxxx/
> for the newly added "ti,am62p-cpsw-mac-efuse" compatible.
> 
> v1:
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402094200.4036076-1-s-vadapalli@xxxxxx/
> Changes since v1:
> - Since "wkup_conf" is modelled as a "simple-bus" rather than being

And maybe the hardware representation is not correct? What bus is it?

>   modelled as a System Controller node with the "syscon" compatible,
>   directly passing the reference to the "wkup_conf" node using the
>   "ti,syscon-efuse" device-tree property will not work.
>   Therefore, I posted the patch at:
>   https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240402105708.4114146-1-s-vadapalli@xxxxxx/
>   in order to add a new compatible to be used for modelling the
>   CTRLMMR_MAC_IDx registers as System Controller nodes, thereby
>   allowing the existing "ti,syscon-efuse" property to be used.
>   Now, "ti,syscon-efuse" points to the "cpsw_mac_efuse" node within
>   "wkup_conf" node, with "cpsw_mac_efuse" being a "syscon" node.
> 
> Logs verifying that the CPSW driver assigns the MAC Address from the
> eFuse based on the CTRLMMR_MAC_IDx registers at 0x43000200 and 0x43000204
> to the interface eth0 corresponding to CPSW3G MAC Port 1:
> https://gist.github.com/Siddharth-Vadapalli-at-TI/9982c6f13bf9b8cfaf97e8517e7dea13
> 
> Regards,
> Siddharth.
> 
>  arch/arm64/boot/dts/ti/k3-am62p-main.dtsi   | 1 +
>  arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> index 7337a9e13535..848ca454a411 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62p-main.dtsi
> @@ -696,6 +696,7 @@ cpsw_port1: port@1 {
>  				label = "port1";
>  				phys = <&phy_gmii_sel 1>;
>  				mac-address = [00 00 00 00 00 00];
> +				ti,syscon-efuse = <&cpsw_mac_efuse 0x0>;

Why this is not nvmem cell, like or efuses?

>  			};
>  
>  			cpsw_port2: port@2 {
> diff --git a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> index a84756c336d0..df9d40f64e3b 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62p-wakeup.dtsi
> @@ -18,6 +18,11 @@ chipid: chipid@14 {
>  			reg = <0x14 0x4>;
>  			bootph-all;
>  		};
> +
> +		cpsw_mac_efuse: cpsw-mac-efuse@200 {

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,am62p-cpsw-mac-efuse", "syscon";
> +			reg = <0x200 0x8>;
> +		};
>  	};
>  
>  	wkup_uart0: serial@2b300000 {

Best regards,
Krzysztof