Hi,
On Mon, Jun 5, 2023 at 12:58 PM Rafał Miłecki <zajec5@xxxxxxxxx> wrote:
> I've include "BCM5301X: " in the subject line.
>
> See below too.
Thank you.
> On 3.06.2023 16:16, Christian Lamparter wrote:
> > fixes the "duplex-full" typo, adds phy-modes for the internal
> > switch and the PHY-chip. This also includs adding pause support
> > for the internal cpu port. Furthermore, both erronous unit properties
> > in the gpio-keys node are removed (#size-cells, #address-cells don't
> > belong here).
> >
> > | ports:port@5:fixed-link: 'oneOf' conditional failed, one must be fixed:
> > | 'anyOf' conditional failed, one must be fixed:
> > | {'speed': [[1000]], 'duplex-full': True} is not of type 'array'
> > | 'duplex-full' does not match any of the regexes
> > | ports:port@5: 'phy-mode' is a required property
> > | keys: '#address-cells', '#size-cells' do not match any of the regexes:
> > | [...] From schema: gpio-keys.yaml
> >
> > Fixes: 935327a73553 ("ARM: dts: BCM5301X: Add DT for Meraki MR26")
> > Signed-off-by: Christian Lamparter <chunkeey@xxxxxxxxx>
> > ---
> > arch/arm/boot/dts/bcm53015-meraki-mr26.dts | 7 ++++---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/bcm53015-meraki-mr26.dts b/arch/arm/boot/dts/bcm53015-meraki-mr26.dts
> > index 9ea4ffc1bb71..9acadf393dd9 100644
> > --- a/arch/arm/boot/dts/bcm53015-meraki-mr26.dts
> > +++ b/arch/arm/boot/dts/bcm53015-meraki-mr26.dts
> > @@ -38,8 +38,6 @@ led-1 {
> >
> > keys {
> > compatible = "gpio-keys";
> > - #address-cells = <1>;
> > - #size-cells = <0>;
>
> FWIW I've already sent patch for that:
> [PATCH 2/2] ARM: dts: BCM5301X: Drop invalid properties from Meraki MR32 keys
Ok, thanks. Was there a mail CCed to me? If it was I have to look where it went.
And yes, that hunk can definitely go.
> > key-restart {
> > label = "Reset";
> > @@ -127,16 +125,19 @@ ports {
> > port@0 {
> > reg = <0>;
> > label = "poe";
> > + phy-mode = "rgmii";
> > };
>
> It was never clear to me how to exactly specify "phy-mode".
>
> It'd values are documented in the:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/net/ethernet-controller.yaml
>
> In Broadcom's bcmrobo.c we can find:
> #define PAGE_GPHY_MII_P0 0x10 /* Port0 Internal GPHY MII registers page */
> #define PAGE_GPHY_MII_P4 0x14 /* Last/Port4 Internal GPHY MII registers page */
>
> That suggests ports 0, 1, 2, 3 and 4 use internal MII.
>
> Does it make "rgmii" a valid value for that?
>From what I remember, the "RGMII" comes simply from the PHY-Chip's
datasheet itself. I "think" that it was a BCM50610.
(The POE is done by a separate TPS23754 chip). The datasheet (
B50610-DS07-R ) boast the following line
in the spec overview: "Supports only RGMII MAC interface" . So by that
logic, it can only be RGMII, since it's the only
mode the PHY-chip accepts.
But let's wait, I see if I can get phytool dump by the weekend, that
should tell me the ID. This can be matched with
the kernel's broadcom phy driver to tell which PHY-chip it is.
>
> Could we just specify a proper value for all 5 ports in the bcm-ns.dtsi?
>
puh, in a dtsi for the whole platform that can stand up to that schema
validation?
Well, I just ask myself: "What can we really set them to, if like
"nothing" is connected?
Are they internally unmuxed, pulled-down or maybe even kept floating?
don't know...".
Is it possible to skip over this question, if all the ports@X are by
default all set to ' status = "disabled" '
in the dtsi? And each device's dts has to set the individual status =
"okay" property to activate them
and then we can expect that the device's dts sets a proper phy-mode
setting? (I mean this is a
established method that all nodes should follow anyway already?)
(don't know if a per-port ' status = "disabled" ' property is
evaluated by the switch drivers/dsa though,
they might just ignore that)
> > port@5 {
> > reg = <5>;
> > label = "cpu";
> > ethernet = <&gmac0>;
> > + phy-mode = "internal";
> >
> > fixed-link {
> > speed = <1000>;
> > - duplex-full;
> > + full-duplex;
> > + pause;
> > };
> > };
> > };
>
> Same here: could we specify "phy-mode" and "fixed-link" for ports 5, 7
> and 8 in the bcm-ns.dtsi? There are more devices with warnings:
>
> DTC_CHK arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp2.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm4708-buffalo-wzr-1166dhp2.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm4708-luxul-xap-1510.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm4708-luxul-xap-1510.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm4708-luxul-xwc-1000.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm4708-netgear-r6250.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm4708-netgear-r6250.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm4708-smartrg-sr400ac.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47081-buffalo-wzr-600dhp2.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47081-luxul-xap-1410.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47081-luxul-xap-1410.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47081-luxul-xwr-1200.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm4709-netgear-r8000.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm4709-netgear-r8000.dtb: ethernet-switch@18007000: ports:port@8: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dtb: ethernet-switch@18007000: ports:port@7: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-asus-rt-ac88u.dtb: ethernet-switch@18007000: ports:port@8: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-dlink-dir-885l.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-dlink-dir-885l.dtb: ethernet-switch@18007000: ports:port@8: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-linksys-panamera.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-linksys-panamera.dtb: ethernet-switch@18007000: ports:port@8: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-linksys-panamera.dtb: switch@0: ports:port@8: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-luxul-abr-4500.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-luxul-abr-4500.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-luxul-xap-1610.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-luxul-xap-1610.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-luxul-xbr-4500.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-luxul-xbr-4500.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-luxul-xwc-2000.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-luxul-xwc-2000.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-luxul-xwr-3100.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-luxul-xwr-3100.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm47094-luxul-xwr-3150-v1.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm47094-luxul-xwr-3150-v1.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm53015-meraki-mr26.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm53015-meraki-mr26.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm53016-meraki-mr32.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm53016-meraki-mr32.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
> DTC_CHK arch/arm/boot/dts/bcm953012er.dtb
> /home/rmilecki/linux/linux-next/arch/arm/boot/dts/bcm953012er.dtb: ethernet-switch@18007000: ports:port@5: 'phy-mode' is a required property
> From schema: /home/rmilecki/linux/linux-next/Documentation/devicetree/bindings/net/dsa/brcm,b53.yaml
I've seen also drivers trying to handle that issue... Now in case of
the MR26 and MR32 this ethernet switch is a
IP-Block inside the SoC right next to the Ethernet-Core. So without
any of broadcom's datasheet about this, I
would just say that the link between the internal switches MAC (on the
"CPU"-port) to the internal
ethernet MAC is as "internal as it gets". (i.e. there's no phy there
at all, it's the one MAC talking directly to the other
MAC and vice versa.)
But for real "switches" chips this is an issue too. Recently, I read
this explanation in a commit write-up for the rtl8365mb switch:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=487994ff75880569d32504d7e70da8b3328e0693
| Since commit a18e6521a7d9 ("net: phylink: handle NA interface mode in
| phylink_fwnode_phy_connect()"), phylib defaults to GMII when no phy-mode
| or phy-connection-type property is specified in a DSA port node of the
| device tree.
| [...]
|
| It should be noted that the aforementioned regression is not because the
| blamed commit was incorrect: on the contrary, the blamed commit is
| correcting the previous behaviour whereby unspecified phy-mode would
| cause the internal interface mode to be PHY_INTERFACE_MODE_NA. The
| rtl8365mb driver only worked by accident before because it _did_
| advertise support for PHY_INTERFACE_MODE_NA, despite NA being reserved
| for internal use by phylink. With one mistake fixed, the other was exposed.
(In a way, phylib's default (whatever they currently are) could be the
answer to that decision?)
Cheers,
Christian
