[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9fea34ee-3dbc-4166-ba7a-a5f4d1551e3d@kernel.org>
Date: Tue, 11 Feb 2025 06:57:17 -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:37, Rabara, Niravkumar L wrote:
> Hi Dinh,
>
>> -----Original Message-----
>> From: Dinh Nguyen <dinguyen@...nel.org>
>> Sent: Tuesday, 11 February, 2025 8:25 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
>>
>> 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, ®_offset);
>>> of_property_read_u32_index(np, "altr,sysmgr-syscon", 2, ®_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?
Powered by blists - more mailing lists