| Message ID | 20231029042712.520010-13-cristian.ciocaltea@collabora.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Conor Dooley |
| Headers | show |
| Series | Enable networking support for StarFive JH7100 SoC | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: > The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > RGMII-ID. > > TODO: Verify if manual adjustment of the RX internal delay is needed. If > yes, add the mdio & phy sub-nodes. Please could you try to get this tested. It might shed some light on what is going on here, since it is a different PHY. Andrew
On 10/29/23 20:46, Andrew Lunn wrote: > On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: >> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >> RGMII-ID. >> >> TODO: Verify if manual adjustment of the RX internal delay is needed. If >> yes, add the mdio & phy sub-nodes. > > Please could you try to get this tested. It might shed some light on > what is going on here, since it is a different PHY. Actually, this is the main reason I added the patch. I don't have access to this board, so it would be great if we could get some help with testing. Thanks, Cristian
On 10/30/23 00:53, Cristian Ciocaltea wrote: > On 10/29/23 20:46, Andrew Lunn wrote: >> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: >>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>> RGMII-ID. >>> >>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>> yes, add the mdio & phy sub-nodes. >> >> Please could you try to get this tested. It might shed some light on >> what is going on here, since it is a different PHY. > > Actually, this is the main reason I added the patch. I don't have access > to this board, so it would be great if we could get some help with testing. @Emil, @Conor: Any idea who might help us with a quick test on the BeagleV Starlight board? Thanks, Cristian
On Thu, Nov 16, 2023 at 03:15:46PM +0200, Cristian Ciocaltea wrote: > On 10/30/23 00:53, Cristian Ciocaltea wrote: > > On 10/29/23 20:46, Andrew Lunn wrote: > >> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: > >>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > >>> RGMII-ID. > >>> > >>> TODO: Verify if manual adjustment of the RX internal delay is needed. If > >>> yes, add the mdio & phy sub-nodes. > >> > >> Please could you try to get this tested. It might shed some light on > >> what is going on here, since it is a different PHY. > > > > Actually, this is the main reason I added the patch. I don't have access > > to this board, so it would be great if we could get some help with testing. > > @Emil, @Conor: Any idea who might help us with a quick test on the > BeagleV Starlight board? I don't have one & I am not sure if Emil does. Geert (CCed) should have one though. Is there a specific test you need to have done?
On 11/16/23 19:55, Conor Dooley wrote: > On Thu, Nov 16, 2023 at 03:15:46PM +0200, Cristian Ciocaltea wrote: >> On 10/30/23 00:53, Cristian Ciocaltea wrote: >>> On 10/29/23 20:46, Andrew Lunn wrote: >>>> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: >>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>>> RGMII-ID. >>>>> >>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>>>> yes, add the mdio & phy sub-nodes. >>>> >>>> Please could you try to get this tested. It might shed some light on >>>> what is going on here, since it is a different PHY. >>> >>> Actually, this is the main reason I added the patch. I don't have access >>> to this board, so it would be great if we could get some help with testing. >> >> @Emil, @Conor: Any idea who might help us with a quick test on the >> BeagleV Starlight board? > > I don't have one & I am not sure if Emil does. Geert (CCed) should have > one though. Is there a specific test you need to have done? As Andrew already pointed out, we'd like to know if networking for this board works without any further adjustment of the RX internal delay. This was necessary for VisionFive (see previous PATCH v2 11/12), but the PHY is different (Motorcomm YT8521), hence this test might help us understand if there's a potential issue with the SoC or the PHY. Thanks, Cristian
On Thu, Nov 16, 2023 at 6:55 PM Conor Dooley <conor@kernel.org> wrote: > On Thu, Nov 16, 2023 at 03:15:46PM +0200, Cristian Ciocaltea wrote: > > On 10/30/23 00:53, Cristian Ciocaltea wrote: > > > On 10/29/23 20:46, Andrew Lunn wrote: > > >> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: > > >>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > > >>> RGMII-ID. > > >>> > > >>> TODO: Verify if manual adjustment of the RX internal delay is needed. If > > >>> yes, add the mdio & phy sub-nodes. > > >> > > >> Please could you try to get this tested. It might shed some light on > > >> what is going on here, since it is a different PHY. > > > > > > Actually, this is the main reason I added the patch. I don't have access > > > to this board, so it would be great if we could get some help with testing. > > > > @Emil, @Conor: Any idea who might help us with a quick test on the > > BeagleV Starlight board? > > I don't have one & I am not sure if Emil does. Geert (CCed) should have I believe Esmil has. > one though. Is there a specific test you need to have done? I gave it a try, on top of latest renesas-drivers[1]. ------------[ cut here ]------------ simple-pm-bus soc: device non-coherent but no non-coherent operations supported WARNING: CPU: 0 PID: 1 at arch/riscv/mm/dma-noncoherent.c:140 arch_setup_dma_ops+0x7e/0xa2 Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.7.0-rc1-starlight-01489-g995c2f0b8b7e #259 Hardware name: BeagleV Starlight Beta (DT) epc : arch_setup_dma_ops+0x7e/0xa2 ra : arch_setup_dma_ops+0x7e/0xa2 epc : ffffffff8000d462 ra : ffffffff8000d462 sp : ffffffc800013b90 gp : ffffffff812d6460 tp : ffffffd880130000 t0 : ffffffff81217238 t1 : 000000000000002d t2 : 2d2d2d2d2d2d2d2d s0 : ffffffc800013bc0 s1 : 0000000000000000 a0 : 000000000000004f a1 : 0000000200000020 a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000 a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000030 s2 : ffffffd880111410 s3 : ffffffff812d712c s4 : 00000000ffffffff s5 : ffffffffffffffed s6 : ffffffd9fffeb530 s7 : ffffffff80a00d70 s8 : ffffffff80e3f2c0 s9 : 0000000000000008 s10: ffffffff808000ba s11: 0000000000000000 t3 : ffffffff812ebb5c t4 : ffffffff812ebb5c t5 : ffffffff812ebb38 t6 : ffffffff812ebbb7 status: 0000000200000120 badaddr: ffffffff812d712c cause: 0000000000000003 [<ffffffff8000d462>] arch_setup_dma_ops+0x7e/0xa2 [<ffffffff80518cb2>] of_dma_configure_id+0xc4/0x222 [<ffffffff803ee76e>] platform_dma_configure+0x44/0x4e [<ffffffff803ec382>] really_probe+0x54/0x20c [<ffffffff803ec596>] __driver_probe_device+0x5c/0xd0 [<ffffffff803ec636>] driver_probe_device+0x2c/0xb0 [<ffffffff803ec7b6>] __driver_attach+0x6e/0x104 [<ffffffff803ea652>] bus_for_each_dev+0x58/0xa4 [<ffffffff803ebdc8>] driver_attach+0x1a/0x22 [<ffffffff803eb7ba>] bus_add_driver+0xd4/0x19e [<ffffffff803ed476>] driver_register+0x3e/0xd8 [<ffffffff803ee590>] __platform_driver_register+0x1c/0x24 [<ffffffff8081db32>] simple_pm_bus_driver_init+0x1a/0x22 [<ffffffff800020f0>] do_one_initcall+0x38/0x17c [<ffffffff80801354>] kernel_init_freeable+0x1a8/0x20c [<ffffffff806b1328>] kernel_init+0x1e/0x112 [<ffffffff806b891e>] ret_from_fork+0xe/0x1c ---[ end trace 0000000000000000 ]--- ------------[ cut here ]------------ clk-starfive-jh7100 11800000.clock-controller: device non-coherent but no non-coherent operations supported WARNING: CPU: 0 PID: 1 at arch/riscv/mm/dma-noncoherent.c:140 arch_setup_dma_ops+0x7e/0xa2 Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Tainted: G S 6.7.0-rc1-starlight-01489-g995c2f0b8b7e #259 Hardware name: BeagleV Starlight Beta (DT) epc : arch_setup_dma_ops+0x7e/0xa2 ra : arch_setup_dma_ops+0x7e/0xa2 epc : ffffffff8000d462 ra : ffffffff8000d462 sp : ffffffc800013b70 gp : ffffffff812d6460 tp : ffffffd880130000 t0 : ffffffff81217f48 t1 : 000000000000002d t2 : 2d2d2d2d2d2d2d2d s0 : ffffffc800013ba0 s1 : 0000000000000000 a0 : 000000000000006b a1 : 0000000200000020 a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000 a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000030 s2 : ffffffd880112410 s3 : ffffffff812d712c s4 : 0000000fffffffff s5 : 0000000000000000 s6 : ffffffd9fffed3c8 s7 : ffffffff80a00d70 s8 : ffffffff80e3f2c0 s9 : 0000000000000008 s10: ffffffff808000ba s11: 0000000000000000 t3 : ffffffff812ec564 t4 : ffffffff812ec564 t5 : ffffffff812ec540 t6 : ffffffff812ec5db status: 0000000200000120 badaddr: ffffffd880130008 cause: 0000000000000003 [<ffffffff8000d462>] arch_setup_dma_ops+0x7e/0xa2 [<ffffffff80518cb2>] of_dma_configure_id+0xc4/0x222 [<ffffffff803ee76e>] platform_dma_configure+0x44/0x4e [<ffffffff803ec382>] really_probe+0x54/0x20c [<ffffffff803ec596>] __driver_probe_device+0x5c/0xd0 [<ffffffff803ec636>] driver_probe_device+0x2c/0xb0 [<ffffffff803ec7b6>] __driver_attach+0x6e/0x104 [<ffffffff803ea652>] bus_for_each_dev+0x58/0xa4 [<ffffffff803ebdc8>] driver_attach+0x1a/0x22 [<ffffffff803eb7ba>] bus_add_driver+0xd4/0x19e [<ffffffff803ed476>] driver_register+0x3e/0xd8 [<ffffffff803ee6ce>] __platform_driver_probe+0x40/0x9c [<ffffffff8081eb80>] clk_starfive_jh7100_driver_init+0x22/0x2a [<ffffffff800020f0>] do_one_initcall+0x38/0x17c [<ffffffff80801354>] kernel_init_freeable+0x1a8/0x20c [<ffffffff806b1328>] kernel_init+0x1e/0x112 [<ffffffff806b891e>] ret_from_fork+0xe/0x1c ---[ end trace 0000000000000000 ]--- CCACHE: DataFail @ 0x00000000.7FEB8930 CCACHE: 2 banks, 16 ways, sets/bank=1024, bytes/block=64 CCACHE: Index of the largest way enabled: 0 ------------[ cut here ]------------ jh7100-reset 11840000.reset-controller: device non-coherent but no non-coherent operations supported WARNING: CPU: 0 PID: 1 at arch/riscv/mm/dma-noncoherent.c:140 arch_setup_dma_ops+0x7e/0xa2 Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Tainted: G S 6.7.0-rc1-starlight-01489-g995c2f0b8b7e #259 Hardware name: BeagleV Starlight Beta (DT) epc : arch_setup_dma_ops+0x7e/0xa2 ra : arch_setup_dma_ops+0x7e/0xa2 epc : ffffffff8000d462 ra : ffffffff8000d462 sp : ffffffc800013b70 gp : ffffffff812d6460 tp : ffffffd880130000 t0 : ffffffff81218d60 t1 : 000000000000002d t2 : 2d2d2d2d2d2d2d2d s0 : ffffffc800013ba0 s1 : 0000000000000000 a0 : 0000000000000064 a1 : 0000000200000020 a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000 a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000030 s2 : ffffffd880112c10 s3 : ffffffff812d712c s4 : 0000000fffffffff s5 : 0000000000000000 s6 : ffffffd9fffed7a8 s7 : ffffffff80a00d70 s8 : ffffffff80e3f2c0 s9 : 0000000000000008 s10: ffffffff808000ba s11: 0000000000000000 t3 : ffffffff812ed054 t4 : ffffffff812ed054 t5 : ffffffff812ed030 t6 : ffffffff812ed0c4 status: 0000000200000120 badaddr: ffffffd880130008 cause: 0000000000000003 [<ffffffff8000d462>] arch_setup_dma_ops+0x7e/0xa2 [<ffffffff80518cb2>] of_dma_configure_id+0xc4/0x222 [<ffffffff803ee76e>] platform_dma_configure+0x44/0x4e [<ffffffff803ec382>] really_probe+0x54/0x20c [<ffffffff803ec596>] __driver_probe_device+0x5c/0xd0 [<ffffffff803ec636>] driver_probe_device+0x2c/0xb0 [<ffffffff803ec7b6>] __driver_attach+0x6e/0x104 [<ffffffff803ea652>] bus_for_each_dev+0x58/0xa4 [<ffffffff803ebdc8>] driver_attach+0x1a/0x22 [<ffffffff803eb7ba>] bus_add_driver+0xd4/0x19e [<ffffffff803ed476>] driver_register+0x3e/0xd8 [<ffffffff803ee6ce>] __platform_driver_probe+0x40/0x9c [<ffffffff8081f284>] jh7100_reset_driver_init+0x22/0x2a [<ffffffff800020f0>] do_one_initcall+0x38/0x17c [<ffffffff80801354>] kernel_init_freeable+0x1a8/0x20c [<ffffffff806b1328>] kernel_init+0x1e/0x112 [<ffffffff806b891e>] ret_from_fork+0xe/0x1c ---[ end trace 0000000000000000 ]--- CCACHE: DataFail @ 0x00000000.7FEB0830 CCACHE: DataFail @ 0x00000000.7FEB07F0 CCACHE: DataFail @ 0x00000000.7FEB07F0 CCACHE: DataFail @ 0x00000000.7FEB07F0 CCACHE: DataFail @ 0x00000000.7FEB0830 CCACHE: DataFail @ 0x00000000.7FEB07F0 [...] Looks like it needs more non-coherent support before we can test Ethernet. Note that it boots fine into Debian nfsroot when merging Emil's[2] visionfive branch instead. [1] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tag/?h=renesas-drivers-2023-11-14-v6.7-rc1 [2] https://github.com/esmil/linux Gr{oetje,eeting}s, Geert
On 11/17/23 10:37, Geert Uytterhoeven wrote: > On Thu, Nov 16, 2023 at 6:55 PM Conor Dooley <conor@kernel.org> wrote: >> On Thu, Nov 16, 2023 at 03:15:46PM +0200, Cristian Ciocaltea wrote: >>> On 10/30/23 00:53, Cristian Ciocaltea wrote: >>>> On 10/29/23 20:46, Andrew Lunn wrote: >>>>> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: >>>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>>>> RGMII-ID. >>>>>> >>>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>>>>> yes, add the mdio & phy sub-nodes. >>>>> >>>>> Please could you try to get this tested. It might shed some light on >>>>> what is going on here, since it is a different PHY. >>>> >>>> Actually, this is the main reason I added the patch. I don't have access >>>> to this board, so it would be great if we could get some help with testing. >>> >>> @Emil, @Conor: Any idea who might help us with a quick test on the >>> BeagleV Starlight board? >> >> I don't have one & I am not sure if Emil does. Geert (CCed) should have > > I believe Esmil has. > >> one though. Is there a specific test you need to have done? > > I gave it a try, on top of latest renesas-drivers[1]. > > ------------[ cut here ]------------ > simple-pm-bus soc: device non-coherent but no non-coherent operations supported > WARNING: CPU: 0 PID: 1 at arch/riscv/mm/dma-noncoherent.c:140 > arch_setup_dma_ops+0x7e/0xa2 > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Not tainted > 6.7.0-rc1-starlight-01489-g995c2f0b8b7e #259 > Hardware name: BeagleV Starlight Beta (DT) > epc : arch_setup_dma_ops+0x7e/0xa2 > ra : arch_setup_dma_ops+0x7e/0xa2 > epc : ffffffff8000d462 ra : ffffffff8000d462 sp : ffffffc800013b90 > gp : ffffffff812d6460 tp : ffffffd880130000 t0 : ffffffff81217238 > t1 : 000000000000002d t2 : 2d2d2d2d2d2d2d2d s0 : ffffffc800013bc0 > s1 : 0000000000000000 a0 : 000000000000004f a1 : 0000000200000020 > a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000 > a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000030 > s2 : ffffffd880111410 s3 : ffffffff812d712c s4 : 00000000ffffffff > s5 : ffffffffffffffed s6 : ffffffd9fffeb530 s7 : ffffffff80a00d70 > s8 : ffffffff80e3f2c0 s9 : 0000000000000008 s10: ffffffff808000ba > s11: 0000000000000000 t3 : ffffffff812ebb5c t4 : ffffffff812ebb5c > t5 : ffffffff812ebb38 t6 : ffffffff812ebbb7 > status: 0000000200000120 badaddr: ffffffff812d712c cause: 0000000000000003 > [<ffffffff8000d462>] arch_setup_dma_ops+0x7e/0xa2 > [<ffffffff80518cb2>] of_dma_configure_id+0xc4/0x222 > [<ffffffff803ee76e>] platform_dma_configure+0x44/0x4e > [<ffffffff803ec382>] really_probe+0x54/0x20c > [<ffffffff803ec596>] __driver_probe_device+0x5c/0xd0 > [<ffffffff803ec636>] driver_probe_device+0x2c/0xb0 > [<ffffffff803ec7b6>] __driver_attach+0x6e/0x104 > [<ffffffff803ea652>] bus_for_each_dev+0x58/0xa4 > [<ffffffff803ebdc8>] driver_attach+0x1a/0x22 > [<ffffffff803eb7ba>] bus_add_driver+0xd4/0x19e > [<ffffffff803ed476>] driver_register+0x3e/0xd8 > [<ffffffff803ee590>] __platform_driver_register+0x1c/0x24 > [<ffffffff8081db32>] simple_pm_bus_driver_init+0x1a/0x22 > [<ffffffff800020f0>] do_one_initcall+0x38/0x17c > [<ffffffff80801354>] kernel_init_freeable+0x1a8/0x20c > [<ffffffff806b1328>] kernel_init+0x1e/0x112 > [<ffffffff806b891e>] ret_from_fork+0xe/0x1c > ---[ end trace 0000000000000000 ]--- > ------------[ cut here ]------------ > clk-starfive-jh7100 11800000.clock-controller: device non-coherent but > no non-coherent operations supported > WARNING: CPU: 0 PID: 1 at arch/riscv/mm/dma-noncoherent.c:140 > arch_setup_dma_ops+0x7e/0xa2 > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Tainted: G S > 6.7.0-rc1-starlight-01489-g995c2f0b8b7e #259 > Hardware name: BeagleV Starlight Beta (DT) > epc : arch_setup_dma_ops+0x7e/0xa2 > ra : arch_setup_dma_ops+0x7e/0xa2 > epc : ffffffff8000d462 ra : ffffffff8000d462 sp : ffffffc800013b70 > gp : ffffffff812d6460 tp : ffffffd880130000 t0 : ffffffff81217f48 > t1 : 000000000000002d t2 : 2d2d2d2d2d2d2d2d s0 : ffffffc800013ba0 > s1 : 0000000000000000 a0 : 000000000000006b a1 : 0000000200000020 > a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000 > a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000030 > s2 : ffffffd880112410 s3 : ffffffff812d712c s4 : 0000000fffffffff > s5 : 0000000000000000 s6 : ffffffd9fffed3c8 s7 : ffffffff80a00d70 > s8 : ffffffff80e3f2c0 s9 : 0000000000000008 s10: ffffffff808000ba > s11: 0000000000000000 t3 : ffffffff812ec564 t4 : ffffffff812ec564 > t5 : ffffffff812ec540 t6 : ffffffff812ec5db > status: 0000000200000120 badaddr: ffffffd880130008 cause: 0000000000000003 > [<ffffffff8000d462>] arch_setup_dma_ops+0x7e/0xa2 > [<ffffffff80518cb2>] of_dma_configure_id+0xc4/0x222 > [<ffffffff803ee76e>] platform_dma_configure+0x44/0x4e > [<ffffffff803ec382>] really_probe+0x54/0x20c > [<ffffffff803ec596>] __driver_probe_device+0x5c/0xd0 > [<ffffffff803ec636>] driver_probe_device+0x2c/0xb0 > [<ffffffff803ec7b6>] __driver_attach+0x6e/0x104 > [<ffffffff803ea652>] bus_for_each_dev+0x58/0xa4 > [<ffffffff803ebdc8>] driver_attach+0x1a/0x22 > [<ffffffff803eb7ba>] bus_add_driver+0xd4/0x19e > [<ffffffff803ed476>] driver_register+0x3e/0xd8 > [<ffffffff803ee6ce>] __platform_driver_probe+0x40/0x9c > [<ffffffff8081eb80>] clk_starfive_jh7100_driver_init+0x22/0x2a > [<ffffffff800020f0>] do_one_initcall+0x38/0x17c > [<ffffffff80801354>] kernel_init_freeable+0x1a8/0x20c > [<ffffffff806b1328>] kernel_init+0x1e/0x112 > [<ffffffff806b891e>] ret_from_fork+0xe/0x1c > ---[ end trace 0000000000000000 ]--- > CCACHE: DataFail @ 0x00000000.7FEB8930 > CCACHE: 2 banks, 16 ways, sets/bank=1024, bytes/block=64 > CCACHE: Index of the largest way enabled: 0 > ------------[ cut here ]------------ > jh7100-reset 11840000.reset-controller: device non-coherent but no > non-coherent operations supported > WARNING: CPU: 0 PID: 1 at arch/riscv/mm/dma-noncoherent.c:140 > arch_setup_dma_ops+0x7e/0xa2 > Modules linked in: > CPU: 0 PID: 1 Comm: swapper/0 Tainted: G S > 6.7.0-rc1-starlight-01489-g995c2f0b8b7e #259 > Hardware name: BeagleV Starlight Beta (DT) > epc : arch_setup_dma_ops+0x7e/0xa2 > ra : arch_setup_dma_ops+0x7e/0xa2 > epc : ffffffff8000d462 ra : ffffffff8000d462 sp : ffffffc800013b70 > gp : ffffffff812d6460 tp : ffffffd880130000 t0 : ffffffff81218d60 > t1 : 000000000000002d t2 : 2d2d2d2d2d2d2d2d s0 : ffffffc800013ba0 > s1 : 0000000000000000 a0 : 0000000000000064 a1 : 0000000200000020 > a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000 > a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000030 > s2 : ffffffd880112c10 s3 : ffffffff812d712c s4 : 0000000fffffffff > s5 : 0000000000000000 s6 : ffffffd9fffed7a8 s7 : ffffffff80a00d70 > s8 : ffffffff80e3f2c0 s9 : 0000000000000008 s10: ffffffff808000ba > s11: 0000000000000000 t3 : ffffffff812ed054 t4 : ffffffff812ed054 > t5 : ffffffff812ed030 t6 : ffffffff812ed0c4 > status: 0000000200000120 badaddr: ffffffd880130008 cause: 0000000000000003 > [<ffffffff8000d462>] arch_setup_dma_ops+0x7e/0xa2 > [<ffffffff80518cb2>] of_dma_configure_id+0xc4/0x222 > [<ffffffff803ee76e>] platform_dma_configure+0x44/0x4e > [<ffffffff803ec382>] really_probe+0x54/0x20c > [<ffffffff803ec596>] __driver_probe_device+0x5c/0xd0 > [<ffffffff803ec636>] driver_probe_device+0x2c/0xb0 > [<ffffffff803ec7b6>] __driver_attach+0x6e/0x104 > [<ffffffff803ea652>] bus_for_each_dev+0x58/0xa4 > [<ffffffff803ebdc8>] driver_attach+0x1a/0x22 > [<ffffffff803eb7ba>] bus_add_driver+0xd4/0x19e > [<ffffffff803ed476>] driver_register+0x3e/0xd8 > [<ffffffff803ee6ce>] __platform_driver_probe+0x40/0x9c > [<ffffffff8081f284>] jh7100_reset_driver_init+0x22/0x2a > [<ffffffff800020f0>] do_one_initcall+0x38/0x17c > [<ffffffff80801354>] kernel_init_freeable+0x1a8/0x20c > [<ffffffff806b1328>] kernel_init+0x1e/0x112 > [<ffffffff806b891e>] ret_from_fork+0xe/0x1c > ---[ end trace 0000000000000000 ]--- > CCACHE: DataFail @ 0x00000000.7FEB0830 > CCACHE: DataFail @ 0x00000000.7FEB07F0 > CCACHE: DataFail @ 0x00000000.7FEB07F0 > CCACHE: DataFail @ 0x00000000.7FEB07F0 > CCACHE: DataFail @ 0x00000000.7FEB0830 > CCACHE: DataFail @ 0x00000000.7FEB07F0 > > [...] > > Looks like it needs more non-coherent support before we can test > Ethernet. Hi Geert, Thanks for taking the time to test this! Could you please check if the following are enabled in your kernel config: CONFIG_DMA_GLOBAL_POOL CONFIG_RISCV_DMA_NONCOHERENT CONFIG_RISCV_NONSTANDARD_CACHE_OPS CONFIG_SIFIVE_CCACHE Regards, Cristian > Note that it boots fine into Debian nfsroot when merging Emil's[2] > visionfive branch instead. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tag/?h=renesas-drivers-2023-11-14-v6.7-rc1 > [2] https://github.com/esmil/linux > > Gr{oetje,eeting}s, > > Geert >
On 11/17/23 10:49, Cristian Ciocaltea wrote: > On 11/17/23 10:37, Geert Uytterhoeven wrote: >> On Thu, Nov 16, 2023 at 6:55 PM Conor Dooley <conor@kernel.org> wrote: >>> On Thu, Nov 16, 2023 at 03:15:46PM +0200, Cristian Ciocaltea wrote: >>>> On 10/30/23 00:53, Cristian Ciocaltea wrote: >>>>> On 10/29/23 20:46, Andrew Lunn wrote: >>>>>> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: >>>>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>>>>> RGMII-ID. >>>>>>> >>>>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>>>>>> yes, add the mdio & phy sub-nodes. >>>>>> >>>>>> Please could you try to get this tested. It might shed some light on >>>>>> what is going on here, since it is a different PHY. >>>>> >>>>> Actually, this is the main reason I added the patch. I don't have access >>>>> to this board, so it would be great if we could get some help with testing. >>>> >>>> @Emil, @Conor: Any idea who might help us with a quick test on the >>>> BeagleV Starlight board? >>> >>> I don't have one & I am not sure if Emil does. Geert (CCed) should have >> >> I believe Esmil has. >> >>> one though. Is there a specific test you need to have done? >> >> I gave it a try, on top of latest renesas-drivers[1]. [...] >> >> Looks like it needs more non-coherent support before we can test >> Ethernet. > > Hi Geert, > > Thanks for taking the time to test this! > > Could you please check if the following are enabled in your kernel config: > > CONFIG_DMA_GLOBAL_POOL > CONFIG_RISCV_DMA_NONCOHERENT > CONFIG_RISCV_NONSTANDARD_CACHE_OPS > CONFIG_SIFIVE_CCACHE Also please note the series requires the SiFive Composable Cache controller patches provided by Emil [1]. [1]: https://lore.kernel.org/all/20231031141444.53426-1-emil.renner.berthing@canonical.com/
Hi Cristian, On Fri, Nov 17, 2023 at 9:59 AM Cristian Ciocaltea <cristian.ciocaltea@collabora.com> wrote: > On 11/17/23 10:49, Cristian Ciocaltea wrote: > > On 11/17/23 10:37, Geert Uytterhoeven wrote: > >> On Thu, Nov 16, 2023 at 6:55 PM Conor Dooley <conor@kernel.org> wrote: > >>> On Thu, Nov 16, 2023 at 03:15:46PM +0200, Cristian Ciocaltea wrote: > >>>> On 10/30/23 00:53, Cristian Ciocaltea wrote: > >>>>> On 10/29/23 20:46, Andrew Lunn wrote: > >>>>>> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: > >>>>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > >>>>>>> RGMII-ID. > >>>>>>> > >>>>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If > >>>>>>> yes, add the mdio & phy sub-nodes. > >>>>>> > >>>>>> Please could you try to get this tested. It might shed some light on > >>>>>> what is going on here, since it is a different PHY. > >>>>> > >>>>> Actually, this is the main reason I added the patch. I don't have access > >>>>> to this board, so it would be great if we could get some help with testing. > >>>> > >>>> @Emil, @Conor: Any idea who might help us with a quick test on the > >>>> BeagleV Starlight board? > >>> > >>> I don't have one & I am not sure if Emil does. Geert (CCed) should have > >> > >> I believe Esmil has. > >> > >>> one though. Is there a specific test you need to have done? > >> > >> I gave it a try, on top of latest renesas-drivers[1]. > > [...] > > >> > >> Looks like it needs more non-coherent support before we can test > >> Ethernet. > > > > Hi Geert, > > > > Thanks for taking the time to test this! > > > > Could you please check if the following are enabled in your kernel config: > > > > CONFIG_DMA_GLOBAL_POOL > > CONFIG_RISCV_DMA_NONCOHERENT > > CONFIG_RISCV_NONSTANDARD_CACHE_OPS > > CONFIG_SIFIVE_CCACHE CONFIG_DMA_GLOBAL_POOL and CONFIG_RISCV_NONSTANDARD_CACHE_OPS were indeed no longer enabled, as they cannot be enabled manually. After cherry-picking commit e14ad9ff67fd51dc ("riscv: errata: Add StarFive JH7100 errata") in esmil/visionfive these options become enabled. Now it gets a bit further, but still lots of CCACHE DataFail errors. > Also please note the series requires the SiFive Composable Cache > controller patches provided by Emil [1]. > > [1]: https://lore.kernel.org/all/20231031141444.53426-1-emil.renner.berthing@canonical.com/ That series does not contain any Kconfig changes, so there must be other missing dependencies? Perhaps I should just defer to Emil ;-) Gr{oetje,eeting}s, Geert
On 11/17/23 11:12, Geert Uytterhoeven wrote: > Hi Cristian, > > On Fri, Nov 17, 2023 at 9:59 AM Cristian Ciocaltea > <cristian.ciocaltea@collabora.com> wrote: >> On 11/17/23 10:49, Cristian Ciocaltea wrote: >>> On 11/17/23 10:37, Geert Uytterhoeven wrote: >>>> On Thu, Nov 16, 2023 at 6:55 PM Conor Dooley <conor@kernel.org> wrote: >>>>> On Thu, Nov 16, 2023 at 03:15:46PM +0200, Cristian Ciocaltea wrote: >>>>>> On 10/30/23 00:53, Cristian Ciocaltea wrote: >>>>>>> On 10/29/23 20:46, Andrew Lunn wrote: >>>>>>>> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: >>>>>>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>>>>>>> RGMII-ID. >>>>>>>>> >>>>>>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>>>>>>>> yes, add the mdio & phy sub-nodes. >>>>>>>> >>>>>>>> Please could you try to get this tested. It might shed some light on >>>>>>>> what is going on here, since it is a different PHY. >>>>>>> >>>>>>> Actually, this is the main reason I added the patch. I don't have access >>>>>>> to this board, so it would be great if we could get some help with testing. >>>>>> >>>>>> @Emil, @Conor: Any idea who might help us with a quick test on the >>>>>> BeagleV Starlight board? >>>>> >>>>> I don't have one & I am not sure if Emil does. Geert (CCed) should have >>>> >>>> I believe Esmil has. >>>> >>>>> one though. Is there a specific test you need to have done? >>>> >>>> I gave it a try, on top of latest renesas-drivers[1]. >> >> [...] >> >>>> >>>> Looks like it needs more non-coherent support before we can test >>>> Ethernet. >>> >>> Hi Geert, >>> >>> Thanks for taking the time to test this! >>> >>> Could you please check if the following are enabled in your kernel config: >>> >>> CONFIG_DMA_GLOBAL_POOL >>> CONFIG_RISCV_DMA_NONCOHERENT >>> CONFIG_RISCV_NONSTANDARD_CACHE_OPS >>> CONFIG_SIFIVE_CCACHE > > CONFIG_DMA_GLOBAL_POOL and CONFIG_RISCV_NONSTANDARD_CACHE_OPS were > indeed no longer enabled, as they cannot be enabled manually. > > After cherry-picking commit e14ad9ff67fd51dc ("riscv: errata: Add > StarFive JH7100 errata") in esmil/visionfive these options become > enabled. Now it gets a bit further, but still lots of CCACHE DataFail > errors. Right, there is an open question [2] in PATCH v2 08/12 if this patch should have been part of Emil's ccache series or I will send it in v3 of my series. [2]: https://lore.kernel.org/lkml/4f661818-1585-41d8-a305-96fd359bc8b8@collabora.com/ >> Also please note the series requires the SiFive Composable Cache >> controller patches provided by Emil [1]. >> >> [1]: https://lore.kernel.org/all/20231031141444.53426-1-emil.renner.berthing@canonical.com/ > > That series does not contain any Kconfig changes, so there must be > other missing dependencies? There shouldn't be any additional Kconfig changes or dependencies as those patches just extend an already existing driver. There were some changes in v2, but they are still compatible with this series (I've retested that to make sure). My tree is based on next-20231024, so I'm going to rebase it onto next-20231117, to exclude the possibility of a regression somewhere. I will also test with renesas-drivers. Thanks, Cristian > Perhaps I should just defer to Emil ;-) > > Gr{oetje,eeting}s, > > Geert >
On 11/17/23 13:19, Cristian Ciocaltea wrote: > On 11/17/23 11:12, Geert Uytterhoeven wrote: >> Hi Cristian, >> >> On Fri, Nov 17, 2023 at 9:59 AM Cristian Ciocaltea >> <cristian.ciocaltea@collabora.com> wrote: >>> On 11/17/23 10:49, Cristian Ciocaltea wrote: >>>> On 11/17/23 10:37, Geert Uytterhoeven wrote: >>>>> On Thu, Nov 16, 2023 at 6:55 PM Conor Dooley <conor@kernel.org> wrote: >>>>>> On Thu, Nov 16, 2023 at 03:15:46PM +0200, Cristian Ciocaltea wrote: >>>>>>> On 10/30/23 00:53, Cristian Ciocaltea wrote: >>>>>>>> On 10/29/23 20:46, Andrew Lunn wrote: >>>>>>>>> On Sun, Oct 29, 2023 at 06:27:12AM +0200, Cristian Ciocaltea wrote: >>>>>>>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>>>>>>>> RGMII-ID. >>>>>>>>>> >>>>>>>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>>>>>>>>> yes, add the mdio & phy sub-nodes. >>>>>>>>> >>>>>>>>> Please could you try to get this tested. It might shed some light on >>>>>>>>> what is going on here, since it is a different PHY. >>>>>>>> >>>>>>>> Actually, this is the main reason I added the patch. I don't have access >>>>>>>> to this board, so it would be great if we could get some help with testing. >>>>>>> >>>>>>> @Emil, @Conor: Any idea who might help us with a quick test on the >>>>>>> BeagleV Starlight board? >>>>>> >>>>>> I don't have one & I am not sure if Emil does. Geert (CCed) should have >>>>> >>>>> I believe Esmil has. >>>>> >>>>>> one though. Is there a specific test you need to have done? >>>>> >>>>> I gave it a try, on top of latest renesas-drivers[1]. >>> >>> [...] >>> >>>>> >>>>> Looks like it needs more non-coherent support before we can test >>>>> Ethernet. >>>> >>>> Hi Geert, >>>> >>>> Thanks for taking the time to test this! >>>> >>>> Could you please check if the following are enabled in your kernel config: >>>> >>>> CONFIG_DMA_GLOBAL_POOL >>>> CONFIG_RISCV_DMA_NONCOHERENT >>>> CONFIG_RISCV_NONSTANDARD_CACHE_OPS >>>> CONFIG_SIFIVE_CCACHE >> >> CONFIG_DMA_GLOBAL_POOL and CONFIG_RISCV_NONSTANDARD_CACHE_OPS were >> indeed no longer enabled, as they cannot be enabled manually. >> >> After cherry-picking commit e14ad9ff67fd51dc ("riscv: errata: Add >> StarFive JH7100 errata") in esmil/visionfive these options become >> enabled. Now it gets a bit further, but still lots of CCACHE DataFail >> errors. > > Right, there is an open question [2] in PATCH v2 08/12 if this patch > should have been part of Emil's ccache series or I will send it in v3 > of my series. > > [2]: https://lore.kernel.org/lkml/4f661818-1585-41d8-a305-96fd359bc8b8@collabora.com/ > >>> Also please note the series requires the SiFive Composable Cache >>> controller patches provided by Emil [1]. >>> >>> [1]: https://lore.kernel.org/all/20231031141444.53426-1-emil.renner.berthing@canonical.com/ >> >> That series does not contain any Kconfig changes, so there must be >> other missing dependencies? > > There shouldn't be any additional Kconfig changes or dependencies as > those patches just extend an already existing driver. There were some > changes in v2, but they are still compatible with this series (I've > retested that to make sure). > > My tree is based on next-20231024, so I'm going to rebase it onto > next-20231117, to exclude the possibility of a regression somewhere. > > I will also test with renesas-drivers. I verified with both trees and didn't notice any issues with my VisionFive board, so I don't really understand why BeagleV Starlight shows a different behavior. For reference, please see [3] which contains all required patches applied on top of next-20231117. The top-most one 9d36dec7e6da ("riscv: dts: starfive: Add JH7100 MMC nodes") is optional, I added it to extend a bit the test suite (SD-card card access also works fine). [3]: https://gitlab.collabora.com/cristicc/linux-next/-/tree/visionfive-eth For configuring the kernel, I used: $ make [...] defconfig $ scripts/config --enable CONFIG_NONPORTABLE --enable ERRATA_STARFIVE_JH7100 I also noticed a warning message right before building starts, but it doesn't seem to be harmful: WARNING: unmet direct dependencies detected for DMA_GLOBAL_POOL Depends on [n]: !ARCH_HAS_DMA_SET_UNCACHED [=n] && !DMA_DIRECT_REMAP [=y] Selected by [y]: - ERRATA_STARFIVE_JH7100 [=y] && ARCH_STARFIVE [=y] && NONPORTABLE [=y] Thanks, Cristian
Cristian Ciocaltea wrote: > The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > RGMII-ID. > > TODO: Verify if manual adjustment of the RX internal delay is needed. If > yes, add the mdio & phy sub-nodes. Sorry for being late here. I've tested that removing the mdio and phy nodes on the the Starlight board works fine, but the rx-internal-delay-ps = <900> property not needed on any of my VisionFive V1 boards either. So I wonder why you need that on your board Also in the driver patch you add support for phy-mode = "rgmii-txid", but here you still set it to "rgmii-id", so which is it? You've alse removed the phy reset gpio on the Starlight board: snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> Why? > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > index 7cda3a89020a..d3f4c99d98da 100644 > --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > @@ -11,3 +11,8 @@ / { > model = "BeagleV Starlight Beta"; > compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; > }; > + > +&gmac { > + phy-mode = "rgmii-id"; > + status = "okay"; > +}; Lastly the phy-mode and status are the same for the VF1 and Starlight boards, so why can't these be set in the jh7100-common.dtsi? /Emil > -- > 2.42.0 >
On 11/26/23 23:10, Emil Renner Berthing wrote: > Cristian Ciocaltea wrote: >> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >> RGMII-ID. >> >> TODO: Verify if manual adjustment of the RX internal delay is needed. If >> yes, add the mdio & phy sub-nodes. > > Sorry for being late here. I've tested that removing the mdio and phy nodes on > the the Starlight board works fine, but the rx-internal-delay-ps = <900> > property not needed on any of my VisionFive V1 boards either. No problem, thanks a lot for taking the time to help with the testing! > So I wonder why you need that on your board I noticed you have a patch 70ca054e82b5 ("net: phy: motorcomm: Disable rgmii rx delay") in your tree, hence I you please confirm the tests were done with that commit reverted? > Also in the driver patch you add support for phy-mode = "rgmii-txid", but here > you still set it to "rgmii-id", so which is it? Please try with "rgmii-id" first. I added "rgmii-txid" to have a fallback solution in case the former cannot be used. > You've alse removed the phy reset gpio on the Starlight board: > > snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> > > Why? I missed this in v1 as the gmac handling was done exclusively in jh7100-common. Thanks for noticing! >> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >> index 7cda3a89020a..d3f4c99d98da 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >> +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >> @@ -11,3 +11,8 @@ / { >> model = "BeagleV Starlight Beta"; >> compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; >> }; >> + >> +&gmac { >> + phy-mode = "rgmii-id"; >> + status = "okay"; >> +}; > > Lastly the phy-mode and status are the same for the VF1 and Starlight boards, > so why can't these be set in the jh7100-common.dtsi? I wasn't sure "rgmii-id" can be used for both boards and I didn't want to unconditionally enable gmac on Starlight before getting a confirmation that this actually works. If there is no way to make it working with "rgmii-id" (w/ or w/o adjusting rx-internal-delay-ps), than we should switch to "rgmii-txid". Thanks, Cristian
Cristian Ciocaltea wrote: > On 11/26/23 23:10, Emil Renner Berthing wrote: > > Cristian Ciocaltea wrote: > >> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > >> RGMII-ID. > >> > >> TODO: Verify if manual adjustment of the RX internal delay is needed. If > >> yes, add the mdio & phy sub-nodes. > > > > Sorry for being late here. I've tested that removing the mdio and phy nodes on > > the the Starlight board works fine, but the rx-internal-delay-ps = <900> > > property not needed on any of my VisionFive V1 boards either. > > No problem, thanks a lot for taking the time to help with the testing! > > > So I wonder why you need that on your board > > I noticed you have a patch 70ca054e82b5 ("net: phy: motorcomm: Disable > rgmii rx delay") in your tree, hence I you please confirm the tests were > done with that commit reverted? > > > Also in the driver patch you add support for phy-mode = "rgmii-txid", but here > > you still set it to "rgmii-id", so which is it? > > Please try with "rgmii-id" first. I added "rgmii-txid" to have a > fallback solution in case the former cannot be used. Ah, I see. Sorry I should have read up on the whole thread. Yes, the Starlight board with the Microchip phy works with "rgmii-id" as is. And you're right, with "rgmii-id" my VF1 needs the rx-internal-delay-ps = <900> property too. > > > You've alse removed the phy reset gpio on the Starlight board: > > > > snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> > > > > Why? > > I missed this in v1 as the gmac handling was done exclusively in > jh7100-common. Thanks for noticing! > > >> > >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > >> --- > >> arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> > >> diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >> index 7cda3a89020a..d3f4c99d98da 100644 > >> --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >> +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >> @@ -11,3 +11,8 @@ / { > >> model = "BeagleV Starlight Beta"; > >> compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; > >> }; > >> + > >> +&gmac { > >> + phy-mode = "rgmii-id"; > >> + status = "okay"; > >> +}; > > > > Lastly the phy-mode and status are the same for the VF1 and Starlight boards, > > so why can't these be set in the jh7100-common.dtsi? > > I wasn't sure "rgmii-id" can be used for both boards and I didn't want > to unconditionally enable gmac on Starlight before getting a > confirmation that this actually works. > > If there is no way to make it working with "rgmii-id" (w/ or w/o > adjusting rx-internal-delay-ps), than we should switch to "rgmii-txid". Yeah, I don't exactly know the difference, but both boards seem to work fine with "rgmii-id", so if that is somehow better and/or more correct let's just go with that. Thanks! /Emil > > Thanks, > Cristian
On 11/28/23 14:08, Emil Renner Berthing wrote: > Cristian Ciocaltea wrote: >> On 11/26/23 23:10, Emil Renner Berthing wrote: >>> Cristian Ciocaltea wrote: >>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>> RGMII-ID. >>>> >>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>>> yes, add the mdio & phy sub-nodes. >>> >>> Sorry for being late here. I've tested that removing the mdio and phy nodes on >>> the the Starlight board works fine, but the rx-internal-delay-ps = <900> >>> property not needed on any of my VisionFive V1 boards either. >> >> No problem, thanks a lot for taking the time to help with the testing! >> >>> So I wonder why you need that on your board >> >> I noticed you have a patch 70ca054e82b5 ("net: phy: motorcomm: Disable >> rgmii rx delay") in your tree, hence I you please confirm the tests were >> done with that commit reverted? >> >>> Also in the driver patch you add support for phy-mode = "rgmii-txid", but here >>> you still set it to "rgmii-id", so which is it? >> >> Please try with "rgmii-id" first. I added "rgmii-txid" to have a >> fallback solution in case the former cannot be used. > > Ah, I see. Sorry I should have read up on the whole thread. Yes, the Starlight > board with the Microchip phy works with "rgmii-id" as is. And you're right, > with "rgmii-id" my VF1 needs the rx-internal-delay-ps = <900> property too. That's great, we have now a pretty clear indication that this uncommon behavior stems from the Motorcomm PHY, and *not* from GMAC. >> >>> You've alse removed the phy reset gpio on the Starlight board: >>> >>> snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> >>> >>> Why? >> >> I missed this in v1 as the gmac handling was done exclusively in >> jh7100-common. Thanks for noticing! >> >>>> >>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >>>> --- >>>> arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>>> >>>> diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>> index 7cda3a89020a..d3f4c99d98da 100644 >>>> --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>> +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>> @@ -11,3 +11,8 @@ / { >>>> model = "BeagleV Starlight Beta"; >>>> compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; >>>> }; >>>> + >>>> +&gmac { >>>> + phy-mode = "rgmii-id"; >>>> + status = "okay"; >>>> +}; >>> >>> Lastly the phy-mode and status are the same for the VF1 and Starlight boards, >>> so why can't these be set in the jh7100-common.dtsi? >> >> I wasn't sure "rgmii-id" can be used for both boards and I didn't want >> to unconditionally enable gmac on Starlight before getting a >> confirmation that this actually works. >> >> If there is no way to make it working with "rgmii-id" (w/ or w/o >> adjusting rx-internal-delay-ps), than we should switch to "rgmii-txid". > > Yeah, I don't exactly know the difference, but both boards seem to work fine > with "rgmii-id", so if that is somehow better and/or more correct let's just go > with that. As Andrew already pointed out, going with "rgmii-id" would be the recommended approach, as this passes the responsibility of adding both TX and RX delays to the PHY. "rgmii-txid" requires the MAC to handle the RX delay, which might break the boards having a conformant (aka well-behaving) PHY. For some reason the Microchip PHY seems to work fine in both cases, but that's most likely an exception, as other PHYs might expose a totally different and undesired behavior. I will prepare a v3 soon, and will drop the patches you have already submitted as part of [1]. Thanks again for your support, Cristian [1]: https://lore.kernel.org/all/20231126232746.264302-1-emil.renner.berthing@canonical.com/
Cristian Ciocaltea wrote: > On 11/28/23 14:08, Emil Renner Berthing wrote: > > Cristian Ciocaltea wrote: > >> On 11/26/23 23:10, Emil Renner Berthing wrote: > >>> Cristian Ciocaltea wrote: > >>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > >>>> RGMII-ID. > >>>> > >>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If > >>>> yes, add the mdio & phy sub-nodes. > >>> > >>> Sorry for being late here. I've tested that removing the mdio and phy nodes on > >>> the the Starlight board works fine, but the rx-internal-delay-ps = <900> > >>> property not needed on any of my VisionFive V1 boards either. > >> > >> No problem, thanks a lot for taking the time to help with the testing! > >> > >>> So I wonder why you need that on your board > >> > >> I noticed you have a patch 70ca054e82b5 ("net: phy: motorcomm: Disable > >> rgmii rx delay") in your tree, hence I you please confirm the tests were > >> done with that commit reverted? > >> > >>> Also in the driver patch you add support for phy-mode = "rgmii-txid", but here > >>> you still set it to "rgmii-id", so which is it? > >> > >> Please try with "rgmii-id" first. I added "rgmii-txid" to have a > >> fallback solution in case the former cannot be used. > > > > Ah, I see. Sorry I should have read up on the whole thread. Yes, the Starlight > > board with the Microchip phy works with "rgmii-id" as is. And you're right, > > with "rgmii-id" my VF1 needs the rx-internal-delay-ps = <900> property too. > > That's great, we have now a pretty clear indication that this uncommon behavior > stems from the Motorcomm PHY, and *not* from GMAC. > > >> > >>> You've alse removed the phy reset gpio on the Starlight board: > >>> > >>> snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> > >>> > >>> Why? > >> > >> I missed this in v1 as the gmac handling was done exclusively in > >> jh7100-common. Thanks for noticing! > >> > >>>> > >>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > >>>> --- > >>>> arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ > >>>> 1 file changed, 5 insertions(+) > >>>> > >>>> diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >>>> index 7cda3a89020a..d3f4c99d98da 100644 > >>>> --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >>>> +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >>>> @@ -11,3 +11,8 @@ / { > >>>> model = "BeagleV Starlight Beta"; > >>>> compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; > >>>> }; > >>>> + > >>>> +&gmac { > >>>> + phy-mode = "rgmii-id"; > >>>> + status = "okay"; > >>>> +}; > >>> > >>> Lastly the phy-mode and status are the same for the VF1 and Starlight boards, > >>> so why can't these be set in the jh7100-common.dtsi? > >> > >> I wasn't sure "rgmii-id" can be used for both boards and I didn't want > >> to unconditionally enable gmac on Starlight before getting a > >> confirmation that this actually works. > >> > >> If there is no way to make it working with "rgmii-id" (w/ or w/o > >> adjusting rx-internal-delay-ps), than we should switch to "rgmii-txid". > > > > Yeah, I don't exactly know the difference, but both boards seem to work fine > > with "rgmii-id", so if that is somehow better and/or more correct let's just go > > with that. > > As Andrew already pointed out, going with "rgmii-id" would be the recommended > approach, as this passes the responsibility of adding both TX and RX delays to > the PHY. "rgmii-txid" requires the MAC to handle the RX delay, which might > break the boards having a conformant (aka well-behaving) PHY. For some reason > the Microchip PHY seems to work fine in both cases, but that's most likely an > exception, as other PHYs might expose a totally different and undesired > behavior. > > I will prepare a v3 soon, and will drop the patches you have already submitted > as part of [1]. Sounds good. Then what's missing for ethernet to work is just the clock patches: https://github.com/esmil/linux/commit/b5abe1cb3815765739aff7949deed6f65b952c4a https://github.com/esmil/linux/commit/3a7a423b15a9f796586cbbdc37010d2b83ff2367 You can either include those as part of your patch series enabling ethernet, or they can be submitted separately with the audio clocks. Either way is fine by me. /Emil > > Thanks again for your support, > Cristian > > [1]: https://lore.kernel.org/all/20231126232746.264302-1-emil.renner.berthing@canonical.com/
On 11/28/23 18:09, Emil Renner Berthing wrote: > Cristian Ciocaltea wrote: >> On 11/28/23 14:08, Emil Renner Berthing wrote: >>> Cristian Ciocaltea wrote: >>>> On 11/26/23 23:10, Emil Renner Berthing wrote: >>>>> Cristian Ciocaltea wrote: >>>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>>>> RGMII-ID. >>>>>> >>>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>>>>> yes, add the mdio & phy sub-nodes. >>>>> >>>>> Sorry for being late here. I've tested that removing the mdio and phy nodes on >>>>> the the Starlight board works fine, but the rx-internal-delay-ps = <900> >>>>> property not needed on any of my VisionFive V1 boards either. >>>> >>>> No problem, thanks a lot for taking the time to help with the testing! >>>> >>>>> So I wonder why you need that on your board >>>> >>>> I noticed you have a patch 70ca054e82b5 ("net: phy: motorcomm: Disable >>>> rgmii rx delay") in your tree, hence I you please confirm the tests were >>>> done with that commit reverted? >>>> >>>>> Also in the driver patch you add support for phy-mode = "rgmii-txid", but here >>>>> you still set it to "rgmii-id", so which is it? >>>> >>>> Please try with "rgmii-id" first. I added "rgmii-txid" to have a >>>> fallback solution in case the former cannot be used. >>> >>> Ah, I see. Sorry I should have read up on the whole thread. Yes, the Starlight >>> board with the Microchip phy works with "rgmii-id" as is. And you're right, >>> with "rgmii-id" my VF1 needs the rx-internal-delay-ps = <900> property too. >> >> That's great, we have now a pretty clear indication that this uncommon behavior >> stems from the Motorcomm PHY, and *not* from GMAC. >> >>>> >>>>> You've alse removed the phy reset gpio on the Starlight board: >>>>> >>>>> snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> >>>>> >>>>> Why? >>>> >>>> I missed this in v1 as the gmac handling was done exclusively in >>>> jh7100-common. Thanks for noticing! >>>> >>>>>> >>>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >>>>>> --- >>>>>> arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ >>>>>> 1 file changed, 5 insertions(+) >>>>>> >>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>>>> index 7cda3a89020a..d3f4c99d98da 100644 >>>>>> --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>>>> @@ -11,3 +11,8 @@ / { >>>>>> model = "BeagleV Starlight Beta"; >>>>>> compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; >>>>>> }; >>>>>> + >>>>>> +&gmac { >>>>>> + phy-mode = "rgmii-id"; >>>>>> + status = "okay"; >>>>>> +}; >>>>> >>>>> Lastly the phy-mode and status are the same for the VF1 and Starlight boards, >>>>> so why can't these be set in the jh7100-common.dtsi? >>>> >>>> I wasn't sure "rgmii-id" can be used for both boards and I didn't want >>>> to unconditionally enable gmac on Starlight before getting a >>>> confirmation that this actually works. >>>> >>>> If there is no way to make it working with "rgmii-id" (w/ or w/o >>>> adjusting rx-internal-delay-ps), than we should switch to "rgmii-txid". >>> >>> Yeah, I don't exactly know the difference, but both boards seem to work fine >>> with "rgmii-id", so if that is somehow better and/or more correct let's just go >>> with that. >> >> As Andrew already pointed out, going with "rgmii-id" would be the recommended >> approach, as this passes the responsibility of adding both TX and RX delays to >> the PHY. "rgmii-txid" requires the MAC to handle the RX delay, which might >> break the boards having a conformant (aka well-behaving) PHY. For some reason >> the Microchip PHY seems to work fine in both cases, but that's most likely an >> exception, as other PHYs might expose a totally different and undesired >> behavior. >> >> I will prepare a v3 soon, and will drop the patches you have already submitted >> as part of [1]. > > Sounds good. Then what's missing for ethernet to work is just the clock patches: > https://github.com/esmil/linux/commit/b5abe1cb3815765739aff7949deed6f65b952c4a > https://github.com/esmil/linux/commit/3a7a423b15a9f796586cbbdc37010d2b83ff2367 > > You can either include those as part of your patch series enabling ethernet, or > they can be submitted separately with the audio clocks. Either way is > fine by me. I can cherry-pick them, but so far I couldn't identify any networking related issues if those patches are not applied. Could it be something specific to Starlight board only? > /Emil > >> >> Thanks again for your support, >> Cristian >> >> [1]: https://lore.kernel.org/all/20231126232746.264302-1-emil.renner.berthing@canonical.com/
Cristian Ciocaltea wrote: > On 11/28/23 18:09, Emil Renner Berthing wrote: > > Cristian Ciocaltea wrote: > >> On 11/28/23 14:08, Emil Renner Berthing wrote: > >>> Cristian Ciocaltea wrote: > >>>> On 11/26/23 23:10, Emil Renner Berthing wrote: > >>>>> Cristian Ciocaltea wrote: > >>>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > >>>>>> RGMII-ID. > >>>>>> > >>>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If > >>>>>> yes, add the mdio & phy sub-nodes. > >>>>> > >>>>> Sorry for being late here. I've tested that removing the mdio and phy nodes on > >>>>> the the Starlight board works fine, but the rx-internal-delay-ps = <900> > >>>>> property not needed on any of my VisionFive V1 boards either. > >>>> > >>>> No problem, thanks a lot for taking the time to help with the testing! > >>>> > >>>>> So I wonder why you need that on your board > >>>> > >>>> I noticed you have a patch 70ca054e82b5 ("net: phy: motorcomm: Disable > >>>> rgmii rx delay") in your tree, hence I you please confirm the tests were > >>>> done with that commit reverted? > >>>> > >>>>> Also in the driver patch you add support for phy-mode = "rgmii-txid", but here > >>>>> you still set it to "rgmii-id", so which is it? > >>>> > >>>> Please try with "rgmii-id" first. I added "rgmii-txid" to have a > >>>> fallback solution in case the former cannot be used. > >>> > >>> Ah, I see. Sorry I should have read up on the whole thread. Yes, the Starlight > >>> board with the Microchip phy works with "rgmii-id" as is. And you're right, > >>> with "rgmii-id" my VF1 needs the rx-internal-delay-ps = <900> property too. > >> > >> That's great, we have now a pretty clear indication that this uncommon behavior > >> stems from the Motorcomm PHY, and *not* from GMAC. > >> > >>>> > >>>>> You've alse removed the phy reset gpio on the Starlight board: > >>>>> > >>>>> snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> > >>>>> > >>>>> Why? > >>>> > >>>> I missed this in v1 as the gmac handling was done exclusively in > >>>> jh7100-common. Thanks for noticing! > >>>> > >>>>>> > >>>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > >>>>>> --- > >>>>>> arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ > >>>>>> 1 file changed, 5 insertions(+) > >>>>>> > >>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >>>>>> index 7cda3a89020a..d3f4c99d98da 100644 > >>>>>> --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts > >>>>>> @@ -11,3 +11,8 @@ / { > >>>>>> model = "BeagleV Starlight Beta"; > >>>>>> compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; > >>>>>> }; > >>>>>> + > >>>>>> +&gmac { > >>>>>> + phy-mode = "rgmii-id"; > >>>>>> + status = "okay"; > >>>>>> +}; > >>>>> > >>>>> Lastly the phy-mode and status are the same for the VF1 and Starlight boards, > >>>>> so why can't these be set in the jh7100-common.dtsi? > >>>> > >>>> I wasn't sure "rgmii-id" can be used for both boards and I didn't want > >>>> to unconditionally enable gmac on Starlight before getting a > >>>> confirmation that this actually works. > >>>> > >>>> If there is no way to make it working with "rgmii-id" (w/ or w/o > >>>> adjusting rx-internal-delay-ps), than we should switch to "rgmii-txid". > >>> > >>> Yeah, I don't exactly know the difference, but both boards seem to work fine > >>> with "rgmii-id", so if that is somehow better and/or more correct let's just go > >>> with that. > >> > >> As Andrew already pointed out, going with "rgmii-id" would be the recommended > >> approach, as this passes the responsibility of adding both TX and RX delays to > >> the PHY. "rgmii-txid" requires the MAC to handle the RX delay, which might > >> break the boards having a conformant (aka well-behaving) PHY. For some reason > >> the Microchip PHY seems to work fine in both cases, but that's most likely an > >> exception, as other PHYs might expose a totally different and undesired > >> behavior. > >> > >> I will prepare a v3 soon, and will drop the patches you have already submitted > >> as part of [1]. > > > > Sounds good. Then what's missing for ethernet to work is just the clock patches: > > https://github.com/esmil/linux/commit/b5abe1cb3815765739aff7949deed6f65b952c4a > > https://github.com/esmil/linux/commit/3a7a423b15a9f796586cbbdc37010d2b83ff2367 > > > > You can either include those as part of your patch series enabling ethernet, or > > they can be submitted separately with the audio clocks. Either way is > > fine by me. > > I can cherry-pick them, but so far I couldn't identify any networking > related issues if those patches are not applied. Could it be something > specific to Starlight board only? No, it's the same for both boards. The dwmac-starfive driver adjusts the tx clock: 1000Mbit -> 125MHz 100Mbit -> 25MHz 10Mbit -> 2.5MHz The tx clock is given in the device tree as the gmac_tx_inv clock which derives from either the gmac_root_div or gmac_rmii_ref external clock like this: gmac_rmii_ref (external) -> gmac_rmii_txclk \ gmac_root_div (500MHz) -> gmac_gtxclk (div N) -> gmac_tx (mux) -> gmac_tx_inv ..where N defaults to 4 and the gmac_tx mux defaults to the gmac_gtxclk, so the gmac_tx_inv clock defaults to 125MHz suitable for 1000Mbit connections. See /sys/kernel/debug/clk/clk_summary for another overview. When the dwmac_starfive driver request to change gmac_tx_inv to 25MHz the clock framework will that it has the CLK_SET_RATE_PARENT flag set, so it will try the gmac_tx clock next. This is a mux that can choose either the 125MHz gmac_gtxclk or the external gmac_rmii_txclk which defaults to 0MHz in the current device trees, so the request cannot be met. That's why we need to set the CLK_SET_RATE_PARENT (and CLK_SET_RATE_NO_REPARENT) flags on the gmac_tx clock so the clock framework again goes to try setting the gmac_gtxclk to 25MHz, which it can because it's a divider and setting N=20 does the trick. On your board you can manually force a 100Mbit connection with ethtool -s eth0 speed 100 That fails on my boards without those two patches. /Emil > >> > >> Thanks again for your support, > >> Cristian > >> > >> [1]: https://lore.kernel.org/all/20231126232746.264302-1-emil.renner.berthing@canonical.com/
On 11/29/23 16:28, Emil Renner Berthing wrote: > Cristian Ciocaltea wrote: >> On 11/28/23 18:09, Emil Renner Berthing wrote: >>> Cristian Ciocaltea wrote: >>>> On 11/28/23 14:08, Emil Renner Berthing wrote: >>>>> Cristian Ciocaltea wrote: >>>>>> On 11/26/23 23:10, Emil Renner Berthing wrote: >>>>>>> Cristian Ciocaltea wrote: >>>>>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>>>>>> RGMII-ID. >>>>>>>> >>>>>>>> TODO: Verify if manual adjustment of the RX internal delay is needed. If >>>>>>>> yes, add the mdio & phy sub-nodes. >>>>>>> >>>>>>> Sorry for being late here. I've tested that removing the mdio and phy nodes on >>>>>>> the the Starlight board works fine, but the rx-internal-delay-ps = <900> >>>>>>> property not needed on any of my VisionFive V1 boards either. >>>>>> >>>>>> No problem, thanks a lot for taking the time to help with the testing! >>>>>> >>>>>>> So I wonder why you need that on your board >>>>>> >>>>>> I noticed you have a patch 70ca054e82b5 ("net: phy: motorcomm: Disable >>>>>> rgmii rx delay") in your tree, hence I you please confirm the tests were >>>>>> done with that commit reverted? >>>>>> >>>>>>> Also in the driver patch you add support for phy-mode = "rgmii-txid", but here >>>>>>> you still set it to "rgmii-id", so which is it? >>>>>> >>>>>> Please try with "rgmii-id" first. I added "rgmii-txid" to have a >>>>>> fallback solution in case the former cannot be used. >>>>> >>>>> Ah, I see. Sorry I should have read up on the whole thread. Yes, the Starlight >>>>> board with the Microchip phy works with "rgmii-id" as is. And you're right, >>>>> with "rgmii-id" my VF1 needs the rx-internal-delay-ps = <900> property too. >>>> >>>> That's great, we have now a pretty clear indication that this uncommon behavior >>>> stems from the Motorcomm PHY, and *not* from GMAC. >>>> >>>>>> >>>>>>> You've alse removed the phy reset gpio on the Starlight board: >>>>>>> >>>>>>> snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> >>>>>>> >>>>>>> Why? >>>>>> >>>>>> I missed this in v1 as the gmac handling was done exclusively in >>>>>> jh7100-common. Thanks for noticing! >>>>>> >>>>>>>> >>>>>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >>>>>>>> --- >>>>>>>> arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ >>>>>>>> 1 file changed, 5 insertions(+) >>>>>>>> >>>>>>>> diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>>>>>> index 7cda3a89020a..d3f4c99d98da 100644 >>>>>>>> --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>>>>>> +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts >>>>>>>> @@ -11,3 +11,8 @@ / { >>>>>>>> model = "BeagleV Starlight Beta"; >>>>>>>> compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; >>>>>>>> }; >>>>>>>> + >>>>>>>> +&gmac { >>>>>>>> + phy-mode = "rgmii-id"; >>>>>>>> + status = "okay"; >>>>>>>> +}; >>>>>>> >>>>>>> Lastly the phy-mode and status are the same for the VF1 and Starlight boards, >>>>>>> so why can't these be set in the jh7100-common.dtsi? >>>>>> >>>>>> I wasn't sure "rgmii-id" can be used for both boards and I didn't want >>>>>> to unconditionally enable gmac on Starlight before getting a >>>>>> confirmation that this actually works. >>>>>> >>>>>> If there is no way to make it working with "rgmii-id" (w/ or w/o >>>>>> adjusting rx-internal-delay-ps), than we should switch to "rgmii-txid". >>>>> >>>>> Yeah, I don't exactly know the difference, but both boards seem to work fine >>>>> with "rgmii-id", so if that is somehow better and/or more correct let's just go >>>>> with that. >>>> >>>> As Andrew already pointed out, going with "rgmii-id" would be the recommended >>>> approach, as this passes the responsibility of adding both TX and RX delays to >>>> the PHY. "rgmii-txid" requires the MAC to handle the RX delay, which might >>>> break the boards having a conformant (aka well-behaving) PHY. For some reason >>>> the Microchip PHY seems to work fine in both cases, but that's most likely an >>>> exception, as other PHYs might expose a totally different and undesired >>>> behavior. >>>> >>>> I will prepare a v3 soon, and will drop the patches you have already submitted >>>> as part of [1]. >>> >>> Sounds good. Then what's missing for ethernet to work is just the clock patches: >>> https://github.com/esmil/linux/commit/b5abe1cb3815765739aff7949deed6f65b952c4a >>> https://github.com/esmil/linux/commit/3a7a423b15a9f796586cbbdc37010d2b83ff2367 >>> >>> You can either include those as part of your patch series enabling ethernet, or >>> they can be submitted separately with the audio clocks. Either way is >>> fine by me. >> >> I can cherry-pick them, but so far I couldn't identify any networking >> related issues if those patches are not applied. Could it be something >> specific to Starlight board only? > > No, it's the same for both boards. The dwmac-starfive driver adjusts > the tx clock: > > 1000Mbit -> 125MHz > 100Mbit -> 25MHz > 10Mbit -> 2.5MHz > > The tx clock is given in the device tree as the gmac_tx_inv clock which derives > from either the gmac_root_div or gmac_rmii_ref external clock like this: > > gmac_rmii_ref (external) -> gmac_rmii_txclk \ > gmac_root_div (500MHz) -> gmac_gtxclk (div N) -> gmac_tx (mux) -> gmac_tx_inv > > ..where N defaults to 4 and the gmac_tx mux defaults to the gmac_gtxclk, so > the gmac_tx_inv clock defaults to 125MHz suitable for 1000Mbit connections. > See /sys/kernel/debug/clk/clk_summary for another overview. > > When the dwmac_starfive driver request to change gmac_tx_inv to 25MHz the clock > framework will that it has the CLK_SET_RATE_PARENT flag set, so it will try > the gmac_tx clock next. This is a mux that can choose either the > 125MHz gmac_gtxclk > or the external gmac_rmii_txclk which defaults to 0MHz in the current > device trees, > so the request cannot be met. > > That's why we need to set the CLK_SET_RATE_PARENT (and CLK_SET_RATE_NO_REPARENT) > flags on the gmac_tx clock so the clock framework again goes to try setting the > gmac_gtxclk to 25MHz, which it can because it's a divider and setting N=20 > does the trick. > > On your board you can manually force a 100Mbit connection with > ethtool -s eth0 speed 100 > > That fails on my boards without those two patches. > /Emil Thanks for the detailed explanation! I've been only verified with gigabit connectivity, that would explain why I didn't notice the issue. I will make sure to properly test this before sending v3. Regards, Cristian
On 11/28/23 02:40, Cristian Ciocaltea wrote: > On 11/26/23 23:10, Emil Renner Berthing wrote: >> Cristian Ciocaltea wrote: >>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>> RGMII-ID. >>> [...] >> You've alse removed the phy reset gpio on the Starlight board: >> >> snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> >> >> Why? > > I missed this in v1 as the gmac handling was done exclusively in > jh7100-common. Thanks for noticing! Hi Emil, I think the reset doesn't actually trigger because "snps,reset-gpios" is not a valid property, it should have been "snps,reset-gpio" (without the trailing "s"). However, this seems to be deprecated now, and the recommended approach would be to define the reset gpio in the phy node, which I did in [1]. Hopefully this won't cause any unexpected behaviour. Otherwise we should probably simply drop it. [1]: https://lore.kernel.org/lkml/20231215204050.2296404-8-cristian.ciocaltea@collabora.com/
Cristian Ciocaltea wrote: > On 11/28/23 02:40, Cristian Ciocaltea wrote: > > On 11/26/23 23:10, Emil Renner Berthing wrote: > >> Cristian Ciocaltea wrote: > >>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting > >>> RGMII-ID. > >>> > > [...] > > >> You've alse removed the phy reset gpio on the Starlight board: > >> > >> snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> > >> > >> Why? > > > > I missed this in v1 as the gmac handling was done exclusively in > > jh7100-common. Thanks for noticing! > > Hi Emil, > > I think the reset doesn't actually trigger because "snps,reset-gpios" is > not a valid property, it should have been "snps,reset-gpio" (without the > trailing "s"). > > However, this seems to be deprecated now, and the recommended approach > would be to define the reset gpio in the phy node, which I did in [1]. > > Hopefully this won't cause any unexpected behaviour. Otherwise we should > probably simply drop it. > > [1]: https://lore.kernel.org/lkml/20231215204050.2296404-8-cristian.ciocaltea@collabora.com/ Oh, nice catch! With your v3 patches the Starlight board still works fine and GPIO63 is correctly grabbed and used for "PHY reset". /Emil
On 12/16/23 21:24, Emil Renner Berthing wrote: > Cristian Ciocaltea wrote: >> On 11/28/23 02:40, Cristian Ciocaltea wrote: >>> On 11/26/23 23:10, Emil Renner Berthing wrote: >>>> Cristian Ciocaltea wrote: >>>>> The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting >>>>> RGMII-ID. >>>>> >> >> [...] >> >>>> You've alse removed the phy reset gpio on the Starlight board: >>>> >>>> snps,reset-gpios = <&gpio 63 GPIO_ACTIVE_LOW> >>>> >>>> Why? >>> >>> I missed this in v1 as the gmac handling was done exclusively in >>> jh7100-common. Thanks for noticing! >> >> Hi Emil, >> >> I think the reset doesn't actually trigger because "snps,reset-gpios" is >> not a valid property, it should have been "snps,reset-gpio" (without the >> trailing "s"). >> >> However, this seems to be deprecated now, and the recommended approach >> would be to define the reset gpio in the phy node, which I did in [1]. >> >> Hopefully this won't cause any unexpected behaviour. Otherwise we should >> probably simply drop it. >> >> [1]: https://lore.kernel.org/lkml/20231215204050.2296404-8-cristian.ciocaltea@collabora.com/ > > Oh, nice catch! With your v3 patches the Starlight board still works fine and > GPIO63 is correctly grabbed and used for "PHY reset". Great, thanks a lot for retesting this!
diff --git a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts index 7cda3a89020a..d3f4c99d98da 100644 --- a/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts +++ b/arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts @@ -11,3 +11,8 @@ / { model = "BeagleV Starlight Beta"; compatible = "beagle,beaglev-starlight-jh7100-r0", "starfive,jh7100"; }; + +&gmac { + phy-mode = "rgmii-id"; + status = "okay"; +};
The BeagleV Starlight SBC uses a Microchip KSZ9031RNXCA PHY supporting RGMII-ID. TODO: Verify if manual adjustment of the RX internal delay is needed. If yes, add the mdio & phy sub-nodes. Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> --- arch/riscv/boot/dts/starfive/jh7100-beaglev-starlight.dts | 5 +++++ 1 file changed, 5 insertions(+)