RE: [PATCH] ARM: dts: socfpga: remove syscon compatible string for sysmgr node

["Rabara, Niravkumar L" <niravkumar.l.rabara@xxxxxxxxx>]

Hi Dinh

> -----Original Message-----
> From: Dinh Nguyen <dinguyen@xxxxxxxxxx>
> Sent: Tuesday, 11 February, 2025 8:07 AM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@xxxxxxxxx>; Rob Herring
> <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Cc: lkp <lkp@xxxxxxxxx>
> Subject: Re: [PATCH] ARM: dts: socfpga: remove syscon compatible string for
> sysmgr node
> 
> On 2/10/25 16:29, Dinh Nguyen wrote:
> > On 1/17/25 09:42, niravkumar.l.rabara@xxxxxxxxx wrote:
> >> From: Niravkumar L Rabara <niravkumar.l.rabara@xxxxxxxxx>
> >>
> >> The SoCFPGA System Manager(sysmgr) dt bindings do not use the syscon
> >> compitible, nor does the Linux system manager driver rely on it.
> >> Remove "syscon" for Arria5, Arria10 and Cyclon5 sysmgr node and fixed
> >> dtbs_check warnings like:
> >>
> >> socfpga_arria5_socdk.dtb: sysmgr@ffd08000: compatible: 'oneOf'
> >> conditional failed, one must be fixed:
> >>     ['altr,sys-mgr', 'syscon'] is too long
> >>     'altr,sys-mgr-s10' was expected
> >>     'altr,sys-mgr' was expected
> >>     from schema $id:
> >> http://devicetree.org/schemas/soc/altera/altr,sys-mgr.yaml#
> >>
> >> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> >> Closes:
> >> https://lore.kernel.org/oe-kbuild-all/202501102323.Xnte2yhi-lkp@intel
> >> .com/
> >> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@xxxxxxxxx>
> >> ---
> >>   arch/arm/boot/dts/intel/socfpga/socfpga.dtsi         | 2 +-
> >>   arch/arm/boot/dts/intel/socfpga/socfpga_arria10.dtsi | 2 +-
> >>   2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/boot/dts/intel/socfpga/socfpga.dtsi
> >> b/arch/arm/boot/dts/intel/socfpga/socfpga.dtsi
> >> index 35be14150f41..f124fb72e260 100644
> >> --- a/arch/arm/boot/dts/intel/socfpga/socfpga.dtsi
> >> +++ b/arch/arm/boot/dts/intel/socfpga/socfpga.dtsi
> >> @@ -853,7 +853,7 @@ spi1: spi@fff01000 {
> >>           };
> >>           sysmgr: sysmgr@ffd08000 {
> >> -            compatible = "altr,sys-mgr", "syscon";
> >> +            compatible = "altr,sys-mgr";
> >>               reg = <0xffd08000 0x4000>;
> >>           };
> >> diff --git a/arch/arm/boot/dts/intel/socfpga/socfpga_arria10.dtsi
> >> b/arch/arm/boot/dts/intel/socfpga/socfpga_arria10.dtsi
> >> index 6b6e77596ffa..015120fb4b02 100644
> >> --- a/arch/arm/boot/dts/intel/socfpga/socfpga_arria10.dtsi
> >> +++ b/arch/arm/boot/dts/intel/socfpga/socfpga_arria10.dtsi
> >> @@ -792,7 +792,7 @@ scu: snoop-control-unit@ffffc000 {
> >>           };
> >>           sysmgr: sysmgr@ffd06000 {
> >> -            compatible = "altr,sys-mgr", "syscon";
> >> +            compatible = "altr,sys-mgr";
> >>               reg = <0xffd06000 0x300>;
> >>               cpu1-start-addr = <0xffd06230>;
> >>           };
> >
> > Did you test this patch on actual hardware? Unless something has
> > changed in the system manager driver, this will probably cause the system
> hang.
> >
> 
> Actually, it will not fail to boot, but you will see SD/MMC fail if the bootloader did
> not set the clk-phase correctly, or you booted from another source not SD/MMC.
> The SD/MMC driver uses syscon to get access to the system manager to set it's
> clk-phase.
> 

Yes, I have tested this using NFS boot, however I didn't observe any issue with SD/MMC
driver.  

=> fdt print /soc/mmc@ff808000
mmc@ff808000 {
	#address-cells = <0x00000001>;
	#size-cells = <0x00000000>;
	compatible = "altr,socfpga-dw-mshc";
	reg = <0xff808000 0x00001000>;
	interrupts = <0x00000000 0x00000062 0x00000004>;
	fifo-depth = <0x00000400>;
	clocks = <0x0000001a 0x00000024>;
	clock-names = "biu", "ciu";
	resets = <0x00000006 0x00000027>;
	altr,sysmgr-syscon = <0x0000001c 0x00000028 0x00000004>;
	status = "okay";
	cap-sd-highspeed;
	cap-mmc-highspeed;
	broken-cd;
	bus-width = <0x00000004>;
	clk-phase-sd-hs = <0x00000000 0x00000087>;
	phandle = <0x00000029>;
};
=> fdt print /soc/sysmgr@ffd06000
sysmgr@ffd06000 {
	compatible = "altr,sys-mgr";
	reg = <0xffd06000 0x00000300>;
	cpu1-start-addr = <0xffd06230>;
	phandle = <0x0000001c>;
};

.

[    1.095784] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 50000000Hz, actual 50000000HZ div = 0)
[    1.105692] mmc0: new high speed SDHC card at address 0001
[    1.108238] at24 0-0051: supply vcc not found, using dummy regulator
[    1.111817] mmcblk0: mmc0:0001 SD32G 29.1 GiB
[    1.118872] at24 0-0051: 4096 byte 24c32 EEPROM, writable, 32 bytes/write
[    1.129186]  mmcblk0: p1 p2 p3

.

root@arria10:~# ls /dev/mmcblk0*
/dev/mmcblk0    /dev/mmcblk0p1  /dev/mmcblk0p2  /dev/mmcblk0p3
root@arria10:~# mount /dev/mmcblk0p1 /tmp/
root@arria10:~# ls /tmp/
extlinux                         socfpga_arria10_socdk_sdmmc.dtb  zImage
fit_spl_fpga.itb                 u-boot.img


Thanks,
Nirav