Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 19 May 2023 15:32:33 +0200
From:   Marek =?UTF-8?B?QmVow7pu?= <kabel@kernel.org>
To:     Alexis =?UTF-8?B?TG90aG9yw6k=?= <alexis.lothore@bootlin.com>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Richard Cochran <richardcochran@gmail.com>,
        netdev@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, thomas.petazzoni@bootlin.com,
        paul.arola@telus.com, scott.roberts@telus.com
Subject: Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: enable support for
 88E6361 switch
Message-ID: <20230519153233.3fb8a4d1@thinkpad>
In-Reply-To: <7419ffc0-b292-97c4-fee6-610a1a841265@bootlin.com>
References: <20230517203430.448705-1-alexis.lothore@bootlin.com>
        <20230517203430.448705-3-alexis.lothore@bootlin.com>
        <9a836863-c279-490f-a49a-de4db5de9fd4@lunn.ch>
        <ee281c0f-5e8b-8453-08bf-858c5503dc22@bootlin.com>
        <6643e099-7b72-4da2-aba1-521e1a4c961b@lunn.ch>
        <20230519143713.1ac9c7a1@thinkpad>
        <7419ffc0-b292-97c4-fee6-610a1a841265@bootlin.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, 19 May 2023 15:16:57 +0200
Alexis Lothor=C3=A9 <alexis.lothore@bootlin.com> wrote:

> On 5/19/23 14:38, Marek Beh=C3=BAn wrote:
> > On Thu, 18 May 2023 14:58:00 +0200
> > Andrew Lunn <andrew@lunn.ch> wrote:
> >  =20
> >>>>> +	[MV88E6361] =3D {
> >>>>> +		.prod_num =3D MV88E6XXX_PORT_SWITCH_ID_PROD_6361,
> >>>>> +		.family =3D MV88E6XXX_FAMILY_6393,
> >>>>> +		.name =3D "Marvell 88E6361",
> >>>>> +		.num_databases =3D 4096,
> >>>>> +		.num_macs =3D 16384,
> >>>>> +		.num_ports =3D 11,
> >>>>> +		/* Ports 1, 2 and 8 are not routed */
> >>>>> +		.invalid_port_mask =3D BIT(1) | BIT(2) | BIT(8),
> >>>>> +		.num_internal_phys =3D 5,   =20
> >>>>
> >>>> Which ports have internal PHYs? 2, 3, 4, 5, 6, 7 ?  What does
> >>>> mv88e6xxx_phy_is_internal() return for these ports, and
> >>>> mv88e6xxx_get_capsmv88e6xxx_get_caps()? I'm wondering if you actually
> >>>> need to list 8 here?   =20
> >>>
> >>> Indeed there is something wrong here too. I need to tune
> >>> mv88e6393x_phylink_get_caps to reflect 88E6361 differences.
> >>>
> >>> As stated above, port 3 to 7 are the ones with internal PHY.
> >>> For mv88e6xxx_phy_is_internal, I see that it is merely comparing the =
port index
> >>> to the number of internal phys, so in this case it would advertise (w=
rongly)
> >>> that ports 0 to 4 have internal phys.   =20
> >>
> >> Ports 1 and 2 should hopefully be protected by the
> >> invalid_port_mask. It should not even be possible to create those
> >> ports. port 0 is interesting, and possibly currently broken on
> >> 6393. Please take a look at that. =20
> >=20
> > Why would port 0 be broken on 6393x ? =20
> By "broken", I guess Andrew means that if we feed port 0 to
> mv88e6xxx_phy_is_internal, it will return true, which is wrong since ther=
e is no
> internal phy for port 0 on 6393X ?

OK that's true :)