linux-kernel - Re: [PATCH] ARM: dts: socfpga: remove syscon compatible string for sysmgr node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9b4e50cd-abab-4f3c-8450-d721bac5db17@kernel.org>
Date: Tue, 11 Feb 2025 06:24:57 -0600
From: Dinh Nguyen <dinguyen@...nel.org>
To: "Rabara, Niravkumar L" <niravkumar.l.rabara@...el.com>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc: lkp <lkp@...el.com>
Subject: Re: [PATCH] ARM: dts: socfpga: remove syscon compatible string for
 sysmgr node

On 2/11/25 06:18, Rabara, Niravkumar L wrote:
> Hi Dinh,
> 
>> -----Original Message-----
>> From: Dinh Nguyen <dinguyen@...nel.org>
>> Sent: Tuesday, 11 February, 2025 12:15 PM
>> To: Rabara, Niravkumar L <niravkumar.l.rabara@...el.com>; Rob Herring
>> <robh@...nel.org>; Krzysztof Kozlowski <krzk+dt@...nel.org>; Conor Dooley
>> <conor+dt@...nel.org>; devicetree@...r.kernel.org; linux-
>> kernel@...r.kernel.org
>> Cc: lkp <lkp@...el.com>
>> Subject: Re: [PATCH] ARM: dts: socfpga: remove syscon compatible string for
>> sysmgr node
>>
>>> Yes, I have tested this using NFS boot, however I didn't observe any
>>> issue with SD/MMC driver.
>>>
>>> => fdt print /soc/mmc@...08000
>>> mmc@...08000 {
>>> 	#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@...06000
>>> sysmgr@...06000 {
>>> 	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@...ia10:~# ls /dev/mmcblk0*
>>> /dev/mmcblk0    /dev/mmcblk0p1  /dev/mmcblk0p2  /dev/mmcblk0p3
>>> root@...ia10:~# mount /dev/mmcblk0p1 /tmp/ root@...ia10:~# ls /tmp/
>>> extlinux                         socfpga_arria10_socdk_sdmmc.dtb  zImage
>>> fit_spl_fpga.itb                 u-boot.img
>>>
>>>
>>
>> You didn't really test anything. There's a register in the System Manager that has
>> set the SD/MMC clk-phase in U-Boot. So you won't see the failure unless you
>> explicitly change the value in that register and then boot Linux, then you will see
>> the failure. If you look at drivers/mmc/host/dw_mmc-pltfm.c and look at the
>> function dw_mci_socfpga_priv_init(), you can see that work in action. If you
>> remove the syscon property, then that portion of the driver will fail.
>> Also the ethernet driver is using the system manager's to set the correct PHY
>> mode through syscon. I think you need to test this patch more thoroughly.
>>
>> DInh
> 
> Altera System Manager driver (drivers/mfd/altera-sysmgr.c) is enabled
> in "socfpga_defconfig" - i.e. CONFIG_MFD_ALTERA_SYSMGR=y
> 
> So, SoCFPGA always using drivers/mfd/altera-sysmgr.c for System Manager
> register access, not the generic "syscon" drivers/mfd/syscon.c.
> That's why we do not need "syscon" compatible for fall back mechanism.
> 
> 	sysmgr: sysmgr@...08000 {
> -		compatible = "altr,sys-mgr", "syscon";
> +		compatible = "altr,sys-mgr";
>   		reg = <0xffd08000 0x4000>;
>   	};
> 	mmc: mmc@...08000 {
> 		…
> 		altr,sysmgr-syscon = <&sysmgr 0x28 4>;
> 		clk-phase-sd-hs = <0>, <135>;
> 		…
> 	};
> 	gmac0: ethernet@...00000 {
> 		…
> 		altr,sysmgr-syscon = <&sysmgr 0x44 0>;
> 		…
> 	};
> 
> 
> Even the sdmmc driver you mentioned is using "drivers/mfd/altera-sysmgr.c"
> not the generic "syscon" drivers/mfd/syscon.c. Same thing for ethernet driver
> as well.
> 
> dw_mci_socfpga_priv_init()  {
> 	...
> 	rc = of_property_read_variable_u32_array(np, "clk-phase-sd-hs", &clk_phase[0], 2, 0);
> 	if (rc < 0)
> 		return 0;
> 
> 	sys_mgr_base_addr = altr_sysmgr_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
> 	if (IS_ERR(sys_mgr_base_addr)) {
> 		dev_warn(host->dev, "clk-phase-sd-hs was specified, but failed to find altr,sys-mgr regmap!\n");
> 		return 0;
> 	}
> 
> 	of_property_read_u32_index(np, "altr,sysmgr-syscon", 1, &reg_offset);
> 	of_property_read_u32_index(np, "altr,sysmgr-syscon", 2, &reg_shift);
> 	...
> }
> 
> Please let me know if my understanding is incorrect.
> 

But the altera-sysmgr driver is using syscon as the interface:

config MFD_ALTERA_SYSMGR
         bool "Altera SOCFPGA System Manager"
         depends on ARCH_INTEL_SOCFPGA && OF
         select MFD_SYSCON

Can you look at your bootlog and see if you see this message 
""clk-phase-sd-hs was specified, but failed to find altr,sys-mgr regmap!"?

Dinh