Re: [PATCH v2] arm64: dts: ti: k3-am62p: use eFuse MAC Address for CPSW3G Port 1
From: Andrew Davis
Date: Thu Apr 04 2024 - 12:29:13 EST
On 4/4/24 5:00 AM, Krzysztof Kozlowski wrote:
On 04/04/2024 11:12, Siddharth Vadapalli wrote:
On Thu, Apr 04, 2024 at 10:43:04AM +0200, Krzysztof Kozlowski wrote:
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?
I will let Andrew comment on it. Andrew had posted a patch at:
https://lore.kernel.org/r/20240124184722.150615-10-afd@xxxxxx/
to convert an equivalent "main_conf" node for AM62 SoC to "simple-bus"
from the existing "syscon".
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?
Since it belongs to the MMIO register set. You had recommended *not*
using nvmem for such MMIO registers at:
https://lore.kernel.org/r/48902771-5d3b-448a-8a74-ac18fb4f1a86@xxxxxxxxxx/
"nvmem is for non-volatile memory, like OCOTP and eFUSE. This is not for
accessing regular MMIO registers of system-controller..."
Despite the "ti,syscon-efuse" property containing the term "efuse" in its
name, it is reading the CTRLMMR_MAC_IDx MMIO registers. So I assumed that
the existing approach which has been used on all K3 SoCs apart from this
one, will be suitable for this SoC as well.
OK, I totally forgot we discussed this.
Discussed but never finalized, here is the last message[0] but with
no response.
You even asked above, "Why this is not nvmem cell", you should trust
your instincts, this *should* be a NVMEM cell. That is how everyone else
handles eFused MACs, no clue why you want us to use syscon?? We would
have no way forward in removing all our DT check warnings with syscon.
Syscon is a hacky dead-end filled with custom compatible strings like
"ti,am62p-cpsw-mac-efuse" and custom properties like "ti,syscon-efuse".
NVMEM is a standard, forcing us to use TI custom syscon properties will
prevent our DT from working on anything other than Linux (unless we go
manually add support for every TI custom property to every DT using SW,
defeats the whole purpose DT).
Andrew
[0] https://lore.kernel.org/all/e7114cb4-e24f-4e78-a89f-4e2e2e704b8a@xxxxxx/
};
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
I was following the convention that other mfd-syscon compatible nodes
seemed to be using:
https://github.com/torvalds/linux/blob/41bccc98fb7931d63d03f326a746ac4d429c1dd3/arch/arm64/boot/dts/ti/k3-am65-main.dtsi#L502
The node is:
dss_oldi_io_ctrl: dss-oldi-io-ctrl@41e0
corresponding to the compatible:
"ti,am654-dss-oldi-io-ctrl"
which was added by commit:
https://github.com/torvalds/linux/commit/cb523495ee2a5938fbdd30b8a35094d386c55c12
So if that one was wrong, then what? I don't know really what type of
device is it, but just because one contributor called it that way, does
not mean you should keep going. Maybe investigate why that contributor
did not decide to follow Devicetree spec recommendation?
Best regards,
Krzysztof