* [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET @ 2020-05-08 22:32 ` Florian Fainelli 2020-05-09 7:38 ` Geert Uytterhoeven 2020-05-11 7:21 ` Marek Szyprowski 0 siblings, 2 replies; 7+ messages in thread From: Florian Fainelli @ 2020-05-08 22:32 UTC (permalink / raw) To: netdev Cc: nsaenzjulienne, wahrenst, m.szyprowski, Florian Fainelli, David S. Miller, Thomas Gleixner, Tal Gilboa, Michael Chan, Geert Uytterhoeven, Andy Gospodarek, Randy Dunlap, open list The GENET controller on the Raspberry Pi 4 (2711) is typically interfaced with an external Broadcom PHY via a RGMII electrical interface. To make sure that delays are properly configured at the PHY side, ensure that we get a chance to have the dedicated Broadcom PHY driver (CONFIG_BROADCOM_PHY) enabled for this to happen. Fixes: 402482a6a78e ("net: bcmgenet: Clear ID_MODE_DIS in EXT_RGMII_OOB_CTRL when not needed") Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> --- David, I would like Marek to indicate whether he is okay or not with this change. Thanks! drivers/net/ethernet/broadcom/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig index 53055ce5dfd6..8a70b9152f7c 100644 --- a/drivers/net/ethernet/broadcom/Kconfig +++ b/drivers/net/ethernet/broadcom/Kconfig @@ -69,6 +69,7 @@ config BCMGENET select BCM7XXX_PHY select MDIO_BCM_UNIMAC select DIMLIB + imply 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] 7+ messages in thread
* Re: [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET 2020-05-08 22:32 ` [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET Florian Fainelli @ 2020-05-09 7:38 ` Geert Uytterhoeven 2020-05-09 17:12 ` Florian Fainelli 2020-05-11 7:21 ` Marek Szyprowski 1 sibling, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2020-05-09 7:38 UTC (permalink / raw) To: Florian Fainelli Cc: netdev, Nicolas Saenz Julienne, Stefan Wahren, Marek Szyprowski, David S. Miller, Thomas Gleixner, Tal Gilboa, Michael Chan, Andy Gospodarek, Randy Dunlap, open list Hi Florian, Thanks for your patch! On Sat, May 9, 2020 at 12:32 AM Florian Fainelli <f.fainelli@gmail.com> wrote: > The GENET controller on the Raspberry Pi 4 (2711) is typically > interfaced with an external Broadcom PHY via a RGMII electrical > interface. To make sure that delays are properly configured at the PHY > side, ensure that we get a chance to have the dedicated Broadcom PHY > driver (CONFIG_BROADCOM_PHY) enabled for this to happen. I guess it can be interfaced to a different external PHY, too? > Fixes: 402482a6a78e ("net: bcmgenet: Clear ID_MODE_DIS in EXT_RGMII_OOB_CTRL when not needed") > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- a/drivers/net/ethernet/broadcom/Kconfig > +++ b/drivers/net/ethernet/broadcom/Kconfig > @@ -69,6 +69,7 @@ config BCMGENET > select BCM7XXX_PHY > select MDIO_BCM_UNIMAC > select DIMLIB > + imply BROADCOM_PHY if ARCH_BCM2835 Which means support for the BROADCOM_PHY is always included on ARCH_BCM2835, even if a different PHY is used? > help > This driver supports the built-in Ethernet MACs found in the > Broadcom BCM7xxx Set Top Box family chipset. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET 2020-05-09 7:38 ` Geert Uytterhoeven @ 2020-05-09 17:12 ` Florian Fainelli 2020-05-10 10:02 ` Geert Uytterhoeven 0 siblings, 1 reply; 7+ messages in thread From: Florian Fainelli @ 2020-05-09 17:12 UTC (permalink / raw) To: Geert Uytterhoeven Cc: netdev, Nicolas Saenz Julienne, Stefan Wahren, Marek Szyprowski, David S. Miller, Thomas Gleixner, Tal Gilboa, Michael Chan, Andy Gospodarek, Randy Dunlap, open list On 5/9/2020 12:38 AM, Geert Uytterhoeven wrote: > Hi Florian, > > Thanks for your patch! > > On Sat, May 9, 2020 at 12:32 AM Florian Fainelli <f.fainelli@gmail.com> wrote: >> The GENET controller on the Raspberry Pi 4 (2711) is typically >> interfaced with an external Broadcom PHY via a RGMII electrical >> interface. To make sure that delays are properly configured at the PHY >> side, ensure that we get a chance to have the dedicated Broadcom PHY >> driver (CONFIG_BROADCOM_PHY) enabled for this to happen. > > I guess it can be interfaced to a different external PHY, too? Yes, although this has not happened yet to the best of my knowledge. > >> Fixes: 402482a6a78e ("net: bcmgenet: Clear ID_MODE_DIS in EXT_RGMII_OOB_CTRL when not needed") >> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > >> --- a/drivers/net/ethernet/broadcom/Kconfig >> +++ b/drivers/net/ethernet/broadcom/Kconfig >> @@ -69,6 +69,7 @@ config BCMGENET >> select BCM7XXX_PHY >> select MDIO_BCM_UNIMAC >> select DIMLIB >> + imply BROADCOM_PHY if ARCH_BCM2835 > > Which means support for the BROADCOM_PHY is always included > on ARCH_BCM2835, even if a different PHY is used? It is included by default on and can be deselected if needed, which is exactly what we want here, a sane default, but without the inflexibility of "select". > >> help >> This driver supports the built-in Ethernet MACs found in the >> Broadcom BCM7xxx Set Top Box family chipset. > > Gr{oetje,eeting}s, > > Geert > -- Florian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET 2020-05-09 17:12 ` Florian Fainelli @ 2020-05-10 10:02 ` Geert Uytterhoeven 0 siblings, 0 replies; 7+ messages in thread From: Geert Uytterhoeven @ 2020-05-10 10:02 UTC (permalink / raw) To: Florian Fainelli Cc: netdev, Nicolas Saenz Julienne, Stefan Wahren, Marek Szyprowski, David S. Miller, Thomas Gleixner, Tal Gilboa, Michael Chan, Andy Gospodarek, Randy Dunlap, open list Hi Florian, On Sat, May 9, 2020 at 7:12 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > On 5/9/2020 12:38 AM, Geert Uytterhoeven wrote: > > On Sat, May 9, 2020 at 12:32 AM Florian Fainelli <f.fainelli@gmail.com> wrote: > >> The GENET controller on the Raspberry Pi 4 (2711) is typically > >> interfaced with an external Broadcom PHY via a RGMII electrical > >> interface. To make sure that delays are properly configured at the PHY > >> side, ensure that we get a chance to have the dedicated Broadcom PHY > >> driver (CONFIG_BROADCOM_PHY) enabled for this to happen. > > > > I guess it can be interfaced to a different external PHY, too? > > Yes, although this has not happened yet to the best of my knowledge. > > > > >> Fixes: 402482a6a78e ("net: bcmgenet: Clear ID_MODE_DIS in EXT_RGMII_OOB_CTRL when not needed") > >> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > > > >> --- a/drivers/net/ethernet/broadcom/Kconfig > >> +++ b/drivers/net/ethernet/broadcom/Kconfig > >> @@ -69,6 +69,7 @@ config BCMGENET > >> select BCM7XXX_PHY > >> select MDIO_BCM_UNIMAC > >> select DIMLIB > >> + imply BROADCOM_PHY if ARCH_BCM2835 > > > > Which means support for the BROADCOM_PHY is always included > > on ARCH_BCM2835, even if a different PHY is used? > > It is included by default on and can be deselected if needed, which is > exactly what we want here, a sane default, but without the inflexibility > of "select". I stand corrected: I can confirm the "imply" no longer selects the target symbol, but merely changes the default. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET 2020-05-08 22:32 ` [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET Florian Fainelli 2020-05-09 7:38 ` Geert Uytterhoeven @ 2020-05-11 7:21 ` Marek Szyprowski 2020-05-11 18:19 ` Florian Fainelli 1 sibling, 1 reply; 7+ messages in thread From: Marek Szyprowski @ 2020-05-11 7:21 UTC (permalink / raw) To: Florian Fainelli, netdev Cc: nsaenzjulienne, wahrenst, David S. Miller, Thomas Gleixner, Tal Gilboa, Michael Chan, Geert Uytterhoeven, Andy Gospodarek, Randy Dunlap, open list Hi Florian, On 09.05.2020 00:32, Florian Fainelli wrote: > The GENET controller on the Raspberry Pi 4 (2711) is typically > interfaced with an external Broadcom PHY via a RGMII electrical > interface. To make sure that delays are properly configured at the PHY > side, ensure that we get a chance to have the dedicated Broadcom PHY > driver (CONFIG_BROADCOM_PHY) enabled for this to happen. > > Fixes: 402482a6a78e ("net: bcmgenet: Clear ID_MODE_DIS in EXT_RGMII_OOB_CTRL when not needed") > Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- > David, > > I would like Marek to indicate whether he is okay or not with this > change. Thanks! It is better. It fixes the default values for ARM 32bit bcm2835_defconfig and ARM 64bit defconfig, so you can add: Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> There is still an issue there. In case of ARM 64bit, when Genet driver is configured as a module, BROADCOM_PHY is also set to module. When I changed Genet to be built-in, BROADCOM_PHY stayed selected as module. This case doesn't work, as Genet driver is loaded much earlier than the rootfs/initrd/etc is available, thus broadcom phy driver is not loaded at all. It looks that some kind of deferred probe is missing there. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET 2020-05-11 7:21 ` Marek Szyprowski @ 2020-05-11 18:19 ` Florian Fainelli 2020-05-12 6:00 ` Marek Szyprowski 0 siblings, 1 reply; 7+ messages in thread From: Florian Fainelli @ 2020-05-11 18:19 UTC (permalink / raw) To: Marek Szyprowski, netdev Cc: nsaenzjulienne, wahrenst, David S. Miller, Thomas Gleixner, Tal Gilboa, Michael Chan, Geert Uytterhoeven, Andy Gospodarek, Randy Dunlap, open list On 5/11/2020 12:21 AM, Marek Szyprowski wrote: > Hi Florian, > > On 09.05.2020 00:32, Florian Fainelli wrote: >> The GENET controller on the Raspberry Pi 4 (2711) is typically >> interfaced with an external Broadcom PHY via a RGMII electrical >> interface. To make sure that delays are properly configured at the PHY >> side, ensure that we get a chance to have the dedicated Broadcom PHY >> driver (CONFIG_BROADCOM_PHY) enabled for this to happen. >> >> Fixes: 402482a6a78e ("net: bcmgenet: Clear ID_MODE_DIS in EXT_RGMII_OOB_CTRL when not needed") >> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >> --- >> David, >> >> I would like Marek to indicate whether he is okay or not with this >> change. Thanks! > > It is better. It fixes the default values for ARM 32bit > bcm2835_defconfig and ARM 64bit defconfig, so you can add: > > Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > > There is still an issue there. In case of ARM 64bit, when Genet driver > is configured as a module, BROADCOM_PHY is also set to module. When I > changed Genet to be built-in, BROADCOM_PHY stayed selected as module. OK. > This case doesn't work, as Genet driver is loaded much earlier than the > rootfs/initrd/etc is available, thus broadcom phy driver is not loaded > at all. It looks that some kind of deferred probe is missing there. In the absence of a specific PHY driver the Generic PHY driver gets used instead. This is a valid situation as I described in my other email because the boot loader/firmware could have left the PHY properly configured with the expected RGMII delays and configuration such that Linux does not need to be aware of anything. I suppose we could change the GENET driver when running on the 2711 platform to reject the PHY driver being "Generic PHY" on the premise that a specialized driver should be used instead, but that seems a bit too restrictive IMHO. Do you prefer a "select BROADCOM_PHY if ARCH_BCM2835" then? -- Florian ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET 2020-05-11 18:19 ` Florian Fainelli @ 2020-05-12 6:00 ` Marek Szyprowski 0 siblings, 0 replies; 7+ messages in thread From: Marek Szyprowski @ 2020-05-12 6:00 UTC (permalink / raw) To: Florian Fainelli, netdev Cc: nsaenzjulienne, wahrenst, David S. Miller, Thomas Gleixner, Tal Gilboa, Michael Chan, Geert Uytterhoeven, Andy Gospodarek, Randy Dunlap, open list Hi Florian, On 11.05.2020 20:19, Florian Fainelli wrote: > On 5/11/2020 12:21 AM, Marek Szyprowski wrote: >> On 09.05.2020 00:32, Florian Fainelli wrote: >>> The GENET controller on the Raspberry Pi 4 (2711) is typically >>> interfaced with an external Broadcom PHY via a RGMII electrical >>> interface. To make sure that delays are properly configured at the PHY >>> side, ensure that we get a chance to have the dedicated Broadcom PHY >>> driver (CONFIG_BROADCOM_PHY) enabled for this to happen. >>> >>> Fixes: 402482a6a78e ("net: bcmgenet: Clear ID_MODE_DIS in EXT_RGMII_OOB_CTRL when not needed") >>> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> >>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >>> --- >>> David, >>> >>> I would like Marek to indicate whether he is okay or not with this >>> change. Thanks! >> It is better. It fixes the default values for ARM 32bit >> bcm2835_defconfig and ARM 64bit defconfig, so you can add: >> >> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> >> >> There is still an issue there. In case of ARM 64bit, when Genet driver >> is configured as a module, BROADCOM_PHY is also set to module. When I >> changed Genet to be built-in, BROADCOM_PHY stayed selected as module. > OK. > >> This case doesn't work, as Genet driver is loaded much earlier than the >> rootfs/initrd/etc is available, thus broadcom phy driver is not loaded >> at all. It looks that some kind of deferred probe is missing there. > In the absence of a specific PHY driver the Generic PHY driver gets used > instead. This is a valid situation as I described in my other email > because the boot loader/firmware could have left the PHY properly > configured with the expected RGMII delays and configuration such that > Linux does not need to be aware of anything. I suppose we could change > the GENET driver when running on the 2711 platform to reject the PHY > driver being "Generic PHY" on the premise that a specialized driver > should be used instead, but that seems a bit too restrictive IMHO. > > Do you prefer a "select BROADCOM_PHY if ARCH_BCM2835" then? Yes. It solves the issue after switching Genet to be built-in without the need to change the CONFIG_BROADCOM_PHY. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-12 6:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20200508223228eucas1p252dd643b4bedf08126cf6af4788f9b01@eucas1p2.samsung.com> 2020-05-08 22:32 ` [PATCH net] net: broadcom: Imply BROADCOM_PHY for BCMGENET Florian Fainelli 2020-05-09 7:38 ` Geert Uytterhoeven 2020-05-09 17:12 ` Florian Fainelli 2020-05-10 10:02 ` Geert Uytterhoeven 2020-05-11 7:21 ` Marek Szyprowski 2020-05-11 18:19 ` Florian Fainelli 2020-05-12 6:00 ` Marek Szyprowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.