[PATCH v1 2/8] arm: dts: socfpga: n5x: enable DT register settings and update GMAC nodes

[Lok, Chen Huei]

Hi Tien Fong,

On 22/4/2026 5:44 pm, Chee, Tien Fong wrote:
>>
>> Remove the clocks property from the QSPI node in. The QSPI
>
> The sentence ends mid-phrase. It should say "...from the QSPI node in 
> socfpga_n5x_socdk-u-boot.dtsi."
Noted, I will fix the truncated sentence in the commit message.
>
>>   &gmac0 {
>> +    compatible = "altr,socfpga-stmmac", "snps,dwmac-3.74a", 
>> "snps,dwmac";
>> +    reset-names = "stmmaceth", "stmmaceth-ocp";
>>       clocks = <&clkmgr N5X_EMAC0_CLK>;
>> +    clock-names = "stmmaceth";
>> +    status = "okay";
>
>
> Enabling a peripheral in a platform-level dtsi rather than a 
> board-level dtsi is wrong.
>
> Not all N5X board variants will necessarily have GMAC0 active.
>
> The status = "okay" belongs in socfpga_n5x_socdk-u-boot.dtsi
>
Agreed. I will move the GMAC0 status enablement to
socfpga_n5x_socdk-u-boot.dtsi.
>>
>>
>>   &i2c0 {
>> @@ -132,6 +145,29 @@
>>       clocks = <&clkmgr N5X_L4_MAIN_CLK>;
>>   };
>>
>> +&socfpga_l3interconnect_firewall {
>> +    coh_cpu0_bypass_OC_Firewall_main_Firewall at f7100200 {
>
>
> DT node names should follow the generic naming conventions described 
> in the Devicetree Specification: short, descriptive, hyphen-separated 
> names.
>
> These look like register block names copied verbatim from hardware 
> documentation.
>
> They should use names more like firewall at f7100200 or 
> ocram-firewall at f7100200.
>
> Also, the mixed case in OC_Firewall_main_Firewall violates DT naming 
> conventions.
>
Sure. I will address all the above comments as suggested and resend in v2.
>
>
> Best regards,
>
> Tien Fong
>
Best regards,

Lok