* [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 @ 2022-11-25 11:50 YueHaibing 2022-11-25 11:59 ` Arnd Bergmann ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: YueHaibing @ 2022-11-25 11:50 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni, yuehaibing, f.fainelli, arnd, naresh.kamboju Cc: netdev, linux-kernel, gregkh commit 8d820bc9d12b ("net: broadcom: Fix BCMGENET Kconfig") fixes the build that contain 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") and enable BCMGENET=y but PTP_1588_CLOCK_OPTIONAL=m, which otherwise leads to a link failure. However this may trigger a runtime failure. Fix the original issue by propagating the PTP_1588_CLOCK_OPTIONAL dependency of BROADCOM_PHY down to BCMGENET. Fixes: 8d820bc9d12b ("net: broadcom: Fix BCMGENET Kconfig") Fixes: 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> Suggested-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- drivers/net/ethernet/broadcom/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig index 55dfdb34e37b..f4ca0c6c0f51 100644 --- a/drivers/net/ethernet/broadcom/Kconfig +++ b/drivers/net/ethernet/broadcom/Kconfig @@ -71,13 +71,14 @@ config BCM63XX_ENET config BCMGENET tristate "Broadcom GENET internal MAC support" depends on HAS_IOMEM + depends on PTP_1588_CLOCK_OPTIONAL || !ARCH_BCM2835 select MII select PHYLIB select FIXED_PHY select BCM7XXX_PHY select MDIO_BCM_UNIMAC select DIMLIB - select BROADCOM_PHY if (ARCH_BCM2835 && PTP_1588_CLOCK_OPTIONAL) + select BROADCOM_PHY if ARCH_BCM2835 help This driver supports the built-in Ethernet MACs found in the Broadcom BCM7xxx Set Top Box family chipset. -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 2022-11-25 11:50 [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 YueHaibing @ 2022-11-25 11:59 ` Arnd Bergmann 2022-11-29 3:18 ` Jakub Kicinski 2022-12-01 4:50 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 8+ messages in thread From: Arnd Bergmann @ 2022-11-25 11:59 UTC (permalink / raw) To: YueHaibing, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, f.fainelli, Naresh Kamboju Cc: Netdev, linux-kernel, Greg Kroah-Hartman On Fri, Nov 25, 2022, at 12:50, YueHaibing wrote: > commit 8d820bc9d12b ("net: broadcom: Fix BCMGENET Kconfig") fixes the build > that contain 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > and enable BCMGENET=y but PTP_1588_CLOCK_OPTIONAL=m, which otherwise > leads to a link failure. However this may trigger a runtime failure. > > Fix the original issue by propagating the PTP_1588_CLOCK_OPTIONAL dependency > of BROADCOM_PHY down to BCMGENET. > > Fixes: 8d820bc9d12b ("net: broadcom: Fix BCMGENET Kconfig") > Fixes: 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > Suggested-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: YueHaibing <yuehaibing@huawei.com> Thanks for fixing this, Acked-by: Arnd Bergmann <arnd@arndb.de> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 2022-11-25 11:50 [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 YueHaibing 2022-11-25 11:59 ` Arnd Bergmann @ 2022-11-29 3:18 ` Jakub Kicinski 2022-11-29 11:56 ` Arnd Bergmann 2022-12-01 4:50 ` patchwork-bot+netdevbpf 2 siblings, 1 reply; 8+ messages in thread From: Jakub Kicinski @ 2022-11-29 3:18 UTC (permalink / raw) To: f.fainelli, Richard Cochran, Arnd Bergmann Cc: YueHaibing, davem, edumazet, pabeni, arnd, naresh.kamboju, netdev, linux-kernel, gregkh On Fri, 25 Nov 2022 19:50:03 +0800 YueHaibing wrote: > commit 8d820bc9d12b ("net: broadcom: Fix BCMGENET Kconfig") fixes the build > that contain 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > and enable BCMGENET=y but PTP_1588_CLOCK_OPTIONAL=m, which otherwise > leads to a link failure. However this may trigger a runtime failure. > > Fix the original issue by propagating the PTP_1588_CLOCK_OPTIONAL dependency > of BROADCOM_PHY down to BCMGENET. > > Fixes: 8d820bc9d12b ("net: broadcom: Fix BCMGENET Kconfig") > Fixes: 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > Suggested-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > --- > drivers/net/ethernet/broadcom/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig > index 55dfdb34e37b..f4ca0c6c0f51 100644 > --- a/drivers/net/ethernet/broadcom/Kconfig > +++ b/drivers/net/ethernet/broadcom/Kconfig > @@ -71,13 +71,14 @@ config BCM63XX_ENET > config BCMGENET > tristate "Broadcom GENET internal MAC support" > depends on HAS_IOMEM > + depends on PTP_1588_CLOCK_OPTIONAL || !ARCH_BCM2835 > select MII > select PHYLIB > select FIXED_PHY > select BCM7XXX_PHY > select MDIO_BCM_UNIMAC > select DIMLIB > - select BROADCOM_PHY if (ARCH_BCM2835 && PTP_1588_CLOCK_OPTIONAL) > + select BROADCOM_PHY if ARCH_BCM2835 > help > This driver supports the built-in Ethernet MACs found in the > Broadcom BCM7xxx Set Top Box family chipset. What's the code path that leads to the failure? I want to double check that the driver is handling the PTP registration return codes correctly. IIUC this is a source of misunderstandings in the PTP API. Richard, here's the original report: https://lore.kernel.org/all/CA+G9fYvKfbJHcMZtybf_0Ru3+6fKPg9HwWTOhdCLrOBXMaeG1A@mail.gmail.com/ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 2022-11-29 3:18 ` Jakub Kicinski @ 2022-11-29 11:56 ` Arnd Bergmann 2022-11-29 11:58 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2022-11-29 11:56 UTC (permalink / raw) To: Jakub Kicinski, f.fainelli, Richard Cochran Cc: YueHaibing, David S . Miller, Eric Dumazet, Paolo Abeni, Naresh Kamboju, Netdev, linux-kernel, Greg Kroah-Hartman On Tue, Nov 29, 2022, at 04:18, Jakub Kicinski wrote: > On Fri, 25 Nov 2022 19:50:03 +0800 YueHaibing wrote: >> diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig >> index 55dfdb34e37b..f4ca0c6c0f51 100644 >> --- a/drivers/net/ethernet/broadcom/Kconfig >> +++ b/drivers/net/ethernet/broadcom/Kconfig >> @@ -71,13 +71,14 @@ config BCM63XX_ENET >> config BCMGENET >> tristate "Broadcom GENET internal MAC support" >> depends on HAS_IOMEM >> + depends on PTP_1588_CLOCK_OPTIONAL || !ARCH_BCM2835 >> select MII >> select PHYLIB >> select FIXED_PHY >> select BCM7XXX_PHY >> select MDIO_BCM_UNIMAC >> select DIMLIB >> - select BROADCOM_PHY if (ARCH_BCM2835 && PTP_1588_CLOCK_OPTIONAL) >> + select BROADCOM_PHY if ARCH_BCM2835 >> help >> This driver supports the built-in Ethernet MACs found in the >> Broadcom BCM7xxx Set Top Box family chipset. > > What's the code path that leads to the failure? I want to double check > that the driver is handling the PTP registration return codes correctly. > IIUC this is a source of misunderstandings in the PTP API. > > Richard, here's the original report: > https://lore.kernel.org/all/CA+G9fYvKfbJHcMZtybf_0Ru3+6fKPg9HwWTOhdCLrOBXMaeG1A@mail.gmail.com The original report was for a different bug that resulted in the BROADCOM_PHY driver not being selectable at all. The remaining problem here is this configuration: CONFIG_ARM=y CONFIG_BCM2835=y CONFIG_BCMGENET=y CONFIG_PTP_1588_CLOCK=m CONFIG_PTP_1588_CLOCK_OPTIONAL=m CONFIG_BROADCOM_PHY=m In this case, BCMGENET should 'select BROADCOM_PHY' to make the driver work correctly, but fails to do this because of the dependency. During early boot, this means it cannot access the PHY because that is in a loadable module, despite commit 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") trying to ensure that it could. Note that many other ethernet drivers don't have this particular 'select' statement and just rely on the .config to contain a sensible set of drivers. In particular that is true when running 64-bit kernels on the same chip, which is now the normal configuration. The alternative to YueHaibing's fix would be to just revert 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") and instead change the defconfig file to include the phy driver, as we do elsewhere. Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 2022-11-29 11:56 ` Arnd Bergmann @ 2022-11-29 11:58 ` Arnd Bergmann 2022-11-29 19:12 ` Florian Fainelli 2022-12-01 4:41 ` Jakub Kicinski 0 siblings, 2 replies; 8+ messages in thread From: Arnd Bergmann @ 2022-11-29 11:58 UTC (permalink / raw) To: Jakub Kicinski, Richard Cochran Cc: YueHaibing, David S . Miller, Eric Dumazet, Paolo Abeni, Naresh Kamboju, Netdev, linux-kernel, Greg Kroah-Hartman, Florian Fainelli [Florian's broadcom.com address bounces, adding him to Cc with his gmail address] On Tue, Nov 29, 2022, at 12:56, Arnd Bergmann wrote: > On Tue, Nov 29, 2022, at 04:18, Jakub Kicinski wrote: >> On Fri, 25 Nov 2022 19:50:03 +0800 YueHaibing wrote: >>> diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig >>> index 55dfdb34e37b..f4ca0c6c0f51 100644 >>> --- a/drivers/net/ethernet/broadcom/Kconfig >>> +++ b/drivers/net/ethernet/broadcom/Kconfig >>> @@ -71,13 +71,14 @@ config BCM63XX_ENET >>> config BCMGENET >>> tristate "Broadcom GENET internal MAC support" >>> depends on HAS_IOMEM >>> + depends on PTP_1588_CLOCK_OPTIONAL || !ARCH_BCM2835 >>> select MII >>> select PHYLIB >>> select FIXED_PHY >>> select BCM7XXX_PHY >>> select MDIO_BCM_UNIMAC >>> select DIMLIB >>> - select BROADCOM_PHY if (ARCH_BCM2835 && PTP_1588_CLOCK_OPTIONAL) >>> + select BROADCOM_PHY if ARCH_BCM2835 >>> help >>> This driver supports the built-in Ethernet MACs found in the >>> Broadcom BCM7xxx Set Top Box family chipset. >> >> What's the code path that leads to the failure? I want to double check >> that the driver is handling the PTP registration return codes correctly. >> IIUC this is a source of misunderstandings in the PTP API. >> >> Richard, here's the original report: >> https://lore.kernel.org/all/CA+G9fYvKfbJHcMZtybf_0Ru3+6fKPg9HwWTOhdCLrOBXMaeG1A@mail.gmail.com > > The original report was for a different bug that resulted in the > BROADCOM_PHY driver not being selectable at all. > > The remaining problem here is this configuration: > > CONFIG_ARM=y > CONFIG_BCM2835=y > CONFIG_BCMGENET=y > CONFIG_PTP_1588_CLOCK=m > CONFIG_PTP_1588_CLOCK_OPTIONAL=m > CONFIG_BROADCOM_PHY=m > > In this case, BCMGENET should 'select BROADCOM_PHY' to make the > driver work correctly, but fails to do this because of the > dependency. During early boot, this means it cannot access the > PHY because that is in a loadable module, despite commit > 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > trying to ensure that it could. > > Note that many other ethernet drivers don't have this > particular 'select' statement and just rely on the .config > to contain a sensible set of drivers. In particular that > is true when running 64-bit kernels on the same chip, > which is now the normal configuration. > > The alternative to YueHaibing's fix would be to just revert > 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > and instead change the defconfig file to include the phy driver, > as we do elsewhere. > > Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 2022-11-29 11:58 ` Arnd Bergmann @ 2022-11-29 19:12 ` Florian Fainelli 2022-12-01 4:41 ` Jakub Kicinski 1 sibling, 0 replies; 8+ messages in thread From: Florian Fainelli @ 2022-11-29 19:12 UTC (permalink / raw) To: Arnd Bergmann, Jakub Kicinski, Richard Cochran Cc: YueHaibing, David S . Miller, Eric Dumazet, Paolo Abeni, Naresh Kamboju, Netdev, linux-kernel, Greg Kroah-Hartman On 11/29/22 03:58, Arnd Bergmann wrote: > [Florian's broadcom.com address bounces, adding him to Cc > with his gmail address] Yes, because it's not a valid email address :) it's either lastname, or first.lastname. > > On Tue, Nov 29, 2022, at 12:56, Arnd Bergmann wrote: >> On Tue, Nov 29, 2022, at 04:18, Jakub Kicinski wrote: >>> On Fri, 25 Nov 2022 19:50:03 +0800 YueHaibing wrote: >>>> diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig >>>> index 55dfdb34e37b..f4ca0c6c0f51 100644 >>>> --- a/drivers/net/ethernet/broadcom/Kconfig >>>> +++ b/drivers/net/ethernet/broadcom/Kconfig >>>> @@ -71,13 +71,14 @@ config BCM63XX_ENET >>>> config BCMGENET >>>> tristate "Broadcom GENET internal MAC support" >>>> depends on HAS_IOMEM >>>> + depends on PTP_1588_CLOCK_OPTIONAL || !ARCH_BCM2835 >>>> select MII >>>> select PHYLIB >>>> select FIXED_PHY >>>> select BCM7XXX_PHY >>>> select MDIO_BCM_UNIMAC >>>> select DIMLIB >>>> - select BROADCOM_PHY if (ARCH_BCM2835 && PTP_1588_CLOCK_OPTIONAL) >>>> + select BROADCOM_PHY if ARCH_BCM2835 >>>> help >>>> This driver supports the built-in Ethernet MACs found in the >>>> Broadcom BCM7xxx Set Top Box family chipset. >>> >>> What's the code path that leads to the failure? I want to double check >>> that the driver is handling the PTP registration return codes correctly. >>> IIUC this is a source of misunderstandings in the PTP API. >>> >>> Richard, here's the original report: >>> https://lore.kernel.org/all/CA+G9fYvKfbJHcMZtybf_0Ru3+6fKPg9HwWTOhdCLrOBXMaeG1A@mail.gmail.com >> >> The original report was for a different bug that resulted in the >> BROADCOM_PHY driver not being selectable at all. >> >> The remaining problem here is this configuration: >> >> CONFIG_ARM=y >> CONFIG_BCM2835=y >> CONFIG_BCMGENET=y >> CONFIG_PTP_1588_CLOCK=m >> CONFIG_PTP_1588_CLOCK_OPTIONAL=m >> CONFIG_BROADCOM_PHY=m >> >> In this case, BCMGENET should 'select BROADCOM_PHY' to make the >> driver work correctly, but fails to do this because of the >> dependency. During early boot, this means it cannot access the >> PHY because that is in a loadable module, despite commit >> 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") >> trying to ensure that it could. I don't believe this is the failure mechanism because there is always a fallback to the Generic PHY driver, the issue is that we have configured a specific RGMII mode in the Device Tree that is acted on by both the Ethernet MAC and the PHY in a way that is electrically incompatible unless the proper PHY driver is also enabled such that the RGMII mode is enabled on the PHY side. -- Florian ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 2022-11-29 11:58 ` Arnd Bergmann 2022-11-29 19:12 ` Florian Fainelli @ 2022-12-01 4:41 ` Jakub Kicinski 1 sibling, 0 replies; 8+ messages in thread From: Jakub Kicinski @ 2022-12-01 4:41 UTC (permalink / raw) To: Arnd Bergmann Cc: Richard Cochran, YueHaibing, David S . Miller, Eric Dumazet, Paolo Abeni, Naresh Kamboju, Netdev, linux-kernel, Greg Kroah-Hartman, Florian Fainelli On Tue, 29 Nov 2022 12:58:23 +0100 Arnd Bergmann wrote: > > The original report was for a different bug that resulted in the > > BROADCOM_PHY driver not being selectable at all. > > > > The remaining problem here is this configuration: > > > > CONFIG_ARM=y > > CONFIG_BCM2835=y > > CONFIG_BCMGENET=y > > CONFIG_PTP_1588_CLOCK=m > > CONFIG_PTP_1588_CLOCK_OPTIONAL=m > > CONFIG_BROADCOM_PHY=m > > > > In this case, BCMGENET should 'select BROADCOM_PHY' to make the > > driver work correctly, but fails to do this because of the > > dependency. During early boot, this means it cannot access the > > PHY because that is in a loadable module, despite commit > > 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > > trying to ensure that it could. > > > > Note that many other ethernet drivers don't have this > > particular 'select' statement and just rely on the .config > > to contain a sensible set of drivers. In particular that > > is true when running 64-bit kernels on the same chip, > > which is now the normal configuration. > > > > The alternative to YueHaibing's fix would be to just revert > > 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > > and instead change the defconfig file to include the phy driver, > > as we do elsewhere. Ah, got it now, I think. Alternatively we could flip the 'select' to 'depends on' and make the user do the legwork? Enough brain cycles spend on this simple fix tho, so applying, thanks! :) ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 2022-11-25 11:50 [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 YueHaibing 2022-11-25 11:59 ` Arnd Bergmann 2022-11-29 3:18 ` Jakub Kicinski @ 2022-12-01 4:50 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 8+ messages in thread From: patchwork-bot+netdevbpf @ 2022-12-01 4:50 UTC (permalink / raw) To: YueHaibing Cc: davem, edumazet, kuba, pabeni, f.fainelli, arnd, naresh.kamboju, netdev, linux-kernel, gregkh Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Fri, 25 Nov 2022 19:50:03 +0800 you wrote: > commit 8d820bc9d12b ("net: broadcom: Fix BCMGENET Kconfig") fixes the build > that contain 99addbe31f55 ("net: broadcom: Select BROADCOM_PHY for BCMGENET") > and enable BCMGENET=y but PTP_1588_CLOCK_OPTIONAL=m, which otherwise > leads to a link failure. However this may trigger a runtime failure. > > Fix the original issue by propagating the PTP_1588_CLOCK_OPTIONAL dependency > of BROADCOM_PHY down to BCMGENET. > > [...] Here is the summary with links: - net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 https://git.kernel.org/netdev/net/c/421f8663b3a7 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-12-01 4:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-25 11:50 [PATCH] net: broadcom: Add PTP_1588_CLOCK_OPTIONAL dependency for BCMGENET under ARCH_BCM2835 YueHaibing 2022-11-25 11:59 ` Arnd Bergmann 2022-11-29 3:18 ` Jakub Kicinski 2022-11-29 11:56 ` Arnd Bergmann 2022-11-29 11:58 ` Arnd Bergmann 2022-11-29 19:12 ` Florian Fainelli 2022-12-01 4:41 ` Jakub Kicinski 2022-12-01 4:50 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).