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

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

[niravkumar.l.rabara]

From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>

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@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202501102323.Xnte2yhi-lkp@intel.com/
Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
---
 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>;
 		};
-- 
2.25.1

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

[Dinh Nguyen]

On 1/17/25 09:42, niravkumar.l.rabara@intel.com wrote:
> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> 
> 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@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202501102323.Xnte2yhi-lkp@intel.com/
> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> ---
>   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.

Dinh

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

[Dinh Nguyen]

On 2/10/25 16:29, Dinh Nguyen wrote:
> On 1/17/25 09:42, niravkumar.l.rabara@intel.com wrote:
>> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
>>
>> 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@intel.com>
>> Closes: 
>> https://lore.kernel.org/oe-kbuild-all/202501102323.Xnte2yhi-lkp@intel.com/
>> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
>> ---
>>   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.

Dinh

Dinh

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

[Rabara, Niravkumar L]

Hi Dinh

> -----Original Message-----
> From: Dinh Nguyen <dinguyen@kernel.org>
> Sent: Tuesday, 11 February, 2025 8:07 AM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: lkp <lkp@intel.com>
> 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@intel.com wrote:
> >> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> >>
> >> 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@intel.com>
> >> Closes:
> >> https://lore.kernel.org/oe-kbuild-all/202501102323.Xnte2yhi-lkp@intel
> >> .com/
> >> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> >> ---
> >>   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

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

[Dinh Nguyen]

On 2/10/25 21:18, Rabara, Niravkumar L wrote:
> Hi Dinh
> 
>> -----Original Message-----
>> From: Dinh Nguyen <dinguyen@kernel.org>
>> Sent: Tuesday, 11 February, 2025 8:07 AM
>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>> <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Cc: lkp <lkp@intel.com>
>> 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@intel.com wrote:
>>>> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
>>>>
>>>> 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@intel.com>
>>>> Closes:
>>>> https://lore.kernel.org/oe-kbuild-all/202501102323.Xnte2yhi-lkp@intel
>>>> .com/
>>>> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
>>>> ---
>>>>    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
> 
> 

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

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

[Rabara, Niravkumar L]

Hi Dinh,

> -----Original Message-----
> From: Dinh Nguyen <dinguyen@kernel.org>
> Sent: Tuesday, 11 February, 2025 12:15 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: lkp <lkp@intel.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@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
> >
> >
> 
> 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@ffd08000 {
-		compatible = "altr,sys-mgr", "syscon";
+		compatible = "altr,sys-mgr";
 		reg = <0xffd08000 0x4000>;
 	};
	mmc: mmc@ff808000 {
		…
		altr,sysmgr-syscon = <&sysmgr 0x28 4>;
		clk-phase-sd-hs = <0>, <135>;
		…
	};
	gmac0: ethernet@ff800000 {
		…
		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. 

Thanks,
Nirav

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

[Dinh Nguyen]

On 2/11/25 06:18, Rabara, Niravkumar L wrote:
> Hi Dinh,
> 
>> -----Original Message-----
>> From: Dinh Nguyen <dinguyen@kernel.org>
>> Sent: Tuesday, 11 February, 2025 12:15 PM
>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>> <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Cc: lkp <lkp@intel.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@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
>>>
>>>
>>
>> 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@ffd08000 {
> -		compatible = "altr,sys-mgr", "syscon";
> +		compatible = "altr,sys-mgr";
>   		reg = <0xffd08000 0x4000>;
>   	};
> 	mmc: mmc@ff808000 {
> 		…
> 		altr,sysmgr-syscon = <&sysmgr 0x28 4>;
> 		clk-phase-sd-hs = <0>, <135>;
> 		…
> 	};
> 	gmac0: ethernet@ff800000 {
> 		…
> 		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

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

[Rabara, Niravkumar L]

Hi Dinh,

> -----Original Message-----
> From: Dinh Nguyen <dinguyen@kernel.org>
> Sent: Tuesday, 11 February, 2025 8:25 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: lkp <lkp@intel.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@kernel.org>
> >> Sent: Tuesday, 11 February, 2025 12:15 PM
> >> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
> >> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> >> Dooley <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
> >> kernel@vger.kernel.org
> >> Cc: lkp <lkp@intel.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@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
> >>>
> >>>
> >>
> >> 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@ffd08000 {
> > -		compatible = "altr,sys-mgr", "syscon";
> > +		compatible = "altr,sys-mgr";
> >   		reg = <0xffd08000 0x4000>;
> >   	};
> > 	mmc: mmc@ff808000 {
> > 		…
> > 		altr,sysmgr-syscon = <&sysmgr 0x28 4>;
> > 		clk-phase-sd-hs = <0>, <135>;
> > 		…
> > 	};
> > 	gmac0: ethernet@ff800000 {
> > 		…
> > 		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!"?
> 

No, I do not see this error/warning in boot log. 
" clk-phase-sd-hs was specified, but failed to find altr,sys-mgr regmap!" 

Also I did test by manually changing the clock phase register value in u-boot,
and then boot Linux without "syscon" compatible, and I do not see any error or 
warning and sdmmc and ethernet drivers are working fine.  

=> md.l 0xffd06028 1
ffd06028: 00000003                             ....
=> mw.l 0xffd06028 0x0 

Thanks,
Nirav

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

[Dinh Nguyen]

On 2/11/25 06:37, Rabara, Niravkumar L wrote:
> Hi Dinh,
> 
>> -----Original Message-----
>> From: Dinh Nguyen <dinguyen@kernel.org>
>> Sent: Tuesday, 11 February, 2025 8:25 PM
>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>> <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Cc: lkp <lkp@intel.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@kernel.org>
>>>> Sent: Tuesday, 11 February, 2025 12:15 PM
>>>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
>>>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
>>>> Dooley <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
>>>> kernel@vger.kernel.org
>>>> Cc: lkp <lkp@intel.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@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
>>>>>
>>>>>
>>>>
>>>> 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@ffd08000 {
>>> -		compatible = "altr,sys-mgr", "syscon";
>>> +		compatible = "altr,sys-mgr";
>>>    		reg = <0xffd08000 0x4000>;
>>>    	};
>>> 	mmc: mmc@ff808000 {
>>> 		…
>>> 		altr,sysmgr-syscon = <&sysmgr 0x28 4>;
>>> 		clk-phase-sd-hs = <0>, <135>;
>>> 		…
>>> 	};
>>> 	gmac0: ethernet@ff800000 {
>>> 		…
>>> 		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!"?
>>
> 
> No, I do not see this error/warning in boot log.
> " clk-phase-sd-hs was specified, but failed to find altr,sys-mgr regmap!"
> 
> Also I did test by manually changing the clock phase register value in u-boot,
> and then boot Linux without "syscon" compatible, and I do not see any error or
> warning and sdmmc and ethernet drivers are working fine.
> 
> => md.l 0xffd06028 1
> ffd06028: 00000003                             ....
> => mw.l 0xffd06028 0x0
> 

Can you try an image that removes MFD_SYSCON from the MFD_ALTERA_SYSMGR?

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

[Rabara, Niravkumar L]

Hi Dinh,

> -----Original Message-----
> From: Dinh Nguyen <dinguyen@kernel.org>
> Sent: Tuesday, 11 February, 2025 8:57 PM
> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Cc: lkp <lkp@intel.com>
> Subject: Re: [PATCH] ARM: dts: socfpga: remove syscon compatible string for
> sysmgr node
> 
> On 2/11/25 06:37, Rabara, Niravkumar L wrote:
> > Hi Dinh,
> >
> >> -----Original Message-----
> >> From: Dinh Nguyen <dinguyen@kernel.org>
> >> Sent: Tuesday, 11 February, 2025 8:25 PM
> >> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
> >> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> >> Dooley <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
> >> kernel@vger.kernel.org
> >> Cc: lkp <lkp@intel.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@kernel.org>
> >>>> Sent: Tuesday, 11 February, 2025 12:15 PM
> >>>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob
> >>>> Herring <robh@kernel.org>; Krzysztof Kozlowski
> >>>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
> >>>> devicetree@vger.kernel.org; linux- kernel@vger.kernel.org
> >>>> Cc: lkp <lkp@intel.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@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
> >>>>>
> >>>>>
> >>>>
> >>>> 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@ffd08000 {
> >>> -		compatible = "altr,sys-mgr", "syscon";
> >>> +		compatible = "altr,sys-mgr";
> >>>    		reg = <0xffd08000 0x4000>;
> >>>    	};
> >>> 	mmc: mmc@ff808000 {
> >>> 		…
> >>> 		altr,sysmgr-syscon = <&sysmgr 0x28 4>;
> >>> 		clk-phase-sd-hs = <0>, <135>;
> >>> 		…
> >>> 	};
> >>> 	gmac0: ethernet@ff800000 {
> >>> 		…
> >>> 		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!"?
> >>
> >
> > No, I do not see this error/warning in boot log.
> > " clk-phase-sd-hs was specified, but failed to find altr,sys-mgr regmap!"
> >
> > Also I did test by manually changing the clock phase register value in
> > u-boot, and then boot Linux without "syscon" compatible, and I do not
> > see any error or warning and sdmmc and ethernet drivers are working fine.
> >
> > => md.l 0xffd06028 1
> > ffd06028: 00000003                             ....
> > => mw.l 0xffd06028 0x0
> >
> 
> Can you try an image that removes MFD_SYSCON from the
> MFD_ALTERA_SYSMGR?

Tried image without MFD_SYSCON in MFD_ALTERA_SYSMGR - Kconfig, 
it still works without any error/warning.

Thanks,
Nirav 

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

[Dinh Nguyen]

On 2/11/25 07:48, Rabara, Niravkumar L wrote:
> Hi Dinh,
> 
>> -----Original Message-----
>> From: Dinh Nguyen <dinguyen@kernel.org>
>> Sent: Tuesday, 11 February, 2025 8:57 PM
>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>> <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org
>> Cc: lkp <lkp@intel.com>
>> Subject: Re: [PATCH] ARM: dts: socfpga: remove syscon compatible string for
>> sysmgr node
>>
>> On 2/11/25 06:37, Rabara, Niravkumar L wrote:
>>> Hi Dinh,
>>>
>>>> -----Original Message-----
>>>> From: Dinh Nguyen <dinguyen@kernel.org>
>>>> Sent: Tuesday, 11 February, 2025 8:25 PM
>>>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob Herring
>>>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
>>>> Dooley <conor+dt@kernel.org>; devicetree@vger.kernel.org; linux-
>>>> kernel@vger.kernel.org
>>>> Cc: lkp <lkp@intel.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@kernel.org>
>>>>>> Sent: Tuesday, 11 February, 2025 12:15 PM
>>>>>> To: Rabara, Niravkumar L <niravkumar.l.rabara@intel.com>; Rob
>>>>>> Herring <robh@kernel.org>; Krzysztof Kozlowski
>>>>>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
>>>>>> devicetree@vger.kernel.org; linux- kernel@vger.kernel.org
>>>>>> Cc: lkp <lkp@intel.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@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
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> 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@ffd08000 {
>>>>> -		compatible = "altr,sys-mgr", "syscon";
>>>>> +		compatible = "altr,sys-mgr";
>>>>>     		reg = <0xffd08000 0x4000>;
>>>>>     	};
>>>>> 	mmc: mmc@ff808000 {
>>>>> 		…
>>>>> 		altr,sysmgr-syscon = <&sysmgr 0x28 4>;
>>>>> 		clk-phase-sd-hs = <0>, <135>;
>>>>> 		…
>>>>> 	};
>>>>> 	gmac0: ethernet@ff800000 {
>>>>> 		…
>>>>> 		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!"?
>>>>
>>>
>>> No, I do not see this error/warning in boot log.
>>> " clk-phase-sd-hs was specified, but failed to find altr,sys-mgr regmap!"
>>>
>>> Also I did test by manually changing the clock phase register value in
>>> u-boot, and then boot Linux without "syscon" compatible, and I do not
>>> see any error or warning and sdmmc and ethernet drivers are working fine.
>>>
>>> => md.l 0xffd06028 1
>>> ffd06028: 00000003                             ....
>>> => mw.l 0xffd06028 0x0
>>>
>>
>> Can you try an image that removes MFD_SYSCON from the
>> MFD_ALTERA_SYSMGR?
> 
> Tried image without MFD_SYSCON in MFD_ALTERA_SYSMGR - Kconfig,
> it still works without any error/warning.
> 

Then something has changed and I'd like for you to investigate what 
changed. This whole driver is using syscon, and now it appears that it's 
not? It doesn't make sense to me. Please investigate more.

Dinh