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;
From:   <Jerry.Ray@microchip.com>
To:     <linux@armlinux.org.uk>
CC:     <andrew@lunn.ch>, <f.fainelli@gmail.com>, <olteanv@gmail.com>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <jbe@pengutronix.de>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH net-next v6 6/6] dsa: lan9303: Migrate to PHYLINK
Thread-Topic: [PATCH net-next v6 6/6] dsa: lan9303: Migrate to PHYLINK
Thread-Index: AQHZJHAUgbmW0ofBykmmlKO+Ubpgs66arr+AgAamOBA=
Date:   Mon, 16 Jan 2023 17:22:05 +0000
Message-ID: <MWHPR11MB169301FF4ED0E0BB2305780AEFC19@MWHPR11MB1693.namprd11.prod.outlook.com>
References: <20230109211849.32530-1-jerry.ray@microchip.com>
 <20230109211849.32530-7-jerry.ray@microchip.com>
 <Y7/zlzcyTsF+z0cN@shell.armlinux.org.uk>
In-Reply-To: <Y7/zlzcyTsF+z0cN@shell.armlinux.org.uk>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?/C2ZJd+7uzheOpNj4fxOBqYkb6HzXjtmy82s4m91w/X0svw0IR762fpkwi/R?=
 =?us-ascii?Q?o8TiRXrctRWIMFeL3Yl41Lvk/5sQILcusRaKem1rTghnihsgi8tqCilpxLu6?=
 =?us-ascii?Q?9zPekXY+b5zQsgsaPkSzcN9bOUOLSSj4DjXjbXd48lGLV00p/vAmbJsCKy5t?=
 =?us-ascii?Q?aQiC1FPyUSvSjkA7Bs4xQRpT1Z1IrXeTAimvNPjUurBYbqzRCoIVy8rYijGG?=
 =?us-ascii?Q?FMvkoWO39X63IoF+44diyS44hIsg4TuOwsF/2yXYJli9BCx9H66JN+qkRbBF?=
 =?us-ascii?Q?4xTkSNmI0rJwTDTPuMKmjeZmMivhEg/kKEPC05bo6+/hwmdluSyJPHULq1j0?=
 =?us-ascii?Q?Q9M4UCtKxjdj+bG8jOBPQItTEjwT6Z91jddPeTElfujpid1YJuX8YGirovsa?=
 =?us-ascii?Q?VYS7NelRLRx4+O32gabGdepR7rVU0B6948JMqRGwdY8hKR+JCnT4/S6L6sTj?=
 =?us-ascii?Q?M9oHeg8LSMBZg4CcrjThfttEK0Mu/fNLaCT+ImYI+slfwMRtr5lzcUeGMkJF?=
 =?us-ascii?Q?nxsXFFbvtMSF2ekVdiumKbiOt3wC4RjOkQUcGFcppP6RT+emi32w3KMjgASV?=
 =?us-ascii?Q?ihmIDwaJJkL9KmM4dk6GNpX4R0ca5sRxirKWir1AcG1bTp/lNdvyfjnkwgAg?=
 =?us-ascii?Q?INuZ6avK/ANIS2d9x0jQh8a6PwCw5a4M2JX2eWXmE0gVcG5ldcpcxXdl+sIM?=
 =?us-ascii?Q?znSben0AQv8YHrV2WtLjg9WB82Jd21mmS5rPP+qF8Qi+YNrRYMxc9ryQUfX5?=
 =?us-ascii?Q?f/KrfV/ifdUVbN6YC+Vg+3JyiIecGfGqj++DZQaiEgAYv2z04UYT/LtLMtQg?=
 =?us-ascii?Q?z3jSXUF8sMIpDnfU+j+G6TjDCcgHWGJ9b0cOFPWRsK2u0eh3wowKvXvTOntc?=
 =?us-ascii?Q?x7fWAWDjZI2oCgOzmeSme5CJ8Wpp3DxLjtCSUKQyDamroKUoba47pC/Vtx1P?=
 =?us-ascii?Q?fakkPcoQJkQKtve5nVsI2AjClfW5tsttoP2xoy7GLcfiBpBGx1fFRCR18bGu?=
 =?us-ascii?Q?kfeUaJw0+HBEzP0q3lEDSiN1xvrhdfzWPWubmbeFSX8fFJt3FqHjFKFv5oQZ?=
 =?us-ascii?Q?UjCXsQFzvbzd+aRcupmR9N3Vz2MZLVgFawjWg0qOxXc6nEfbt4vwjvqf6B0r?=
 =?us-ascii?Q?VPh4wGTVzpTeiz7+3JU0qHoyLW/ZFMO60Ru/dIBwYTtSEBftykq3BPd1Fk8L?=
 =?us-ascii?Q?zbGILN0W3yYeie3HzS9x7ipwFhQzWJ9tjP7icwz2slVAKAM7vX7kW2giWh0i?=
 =?us-ascii?Q?q1T9SM5+pJG8NadZa5sJ5HtomjcGElfmysAvccBC9JHlHPEV8sWbDBRYLUv6?=
 =?us-ascii?Q?UuLvKz+EqqqXntCJlGW0byThqIavcSqA66slaBev1sSabph9A3HJ/yXsz7Ip?=
 =?us-ascii?Q?FZ9BDwMNnvwbQrwovGsFiJCUUxznX6UwyVP/SP/n50xqDXt5Rg6qV1NNjkSO?=
 =?us-ascii?Q?lSfRt0WupS/LlIviQzLVIOv9ib/gt+k/EhaBfVmySppirzSs3D0xU7+fWcGw?=
 =?us-ascii?Q?ITQjdf0yInFK+b1+p97QyBD/78GDGVB6J3Mk6RKcGvTnhKPcqevcVn+co7x+?=
 =?us-ascii?Q?xW2ORIxLHynVLiBbliUHOtryMQNY9xhnGxpZpEQN?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: MWHPR11MB1693.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 71916cf6-011c-41f9-d345-08daf7e63077
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Jan 2023 17:22:05.3732
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: FM40zN+nnYjl6DWlxxwf6PhraotVj2EsV+Gpml7jYEnkkOCGMErTXwoaPf+LwbMpvwKN+/hmTJcth4w11KaJJA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4880
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> > +static void lan9303_phylink_get_caps(struct dsa_switch *ds, int port,
> > +                                  struct phylink_config *config)
> > +{
> > +     struct lan9303 *chip =3D ds->priv;
> > +
> > +     dev_dbg(chip->dev, "%s(%d) entered.", __func__, port);
> > +
> > +     config->mac_capabilities =3D MAC_10 | MAC_100 | MAC_ASYM_PAUSE |
> > +                                MAC_SYM_PAUSE;
>=20
> You indicate that pause modes are supported, but...
>=20
> > +static void lan9303_phylink_mac_link_up(struct dsa_switch *ds, int por=
t,
> > +                                     unsigned int mode,
> > +                                     phy_interface_t interface,
> > +                                     struct phy_device *phydev, int sp=
eed,
> > +                                     int duplex, bool tx_pause,
> > +                                     bool rx_pause)
> > +{
> > +     u32 ctl;
> > +
> > +     /* On this device, we are only interested in doing something here=
 if
> > +      * this is the xMII port. All other ports are 10/100 phys using M=
DIO
> > +      * to control there link settings.
> > +      */
> > +     if (port !=3D 0)
> > +             return;
> > +
> > +     ctl =3D lan9303_phy_read(ds, port, MII_BMCR);
> > +
> > +     ctl &=3D ~BMCR_ANENABLE;
> > +
> > +     if (speed =3D=3D SPEED_100)
> > +             ctl |=3D BMCR_SPEED100;
> > +     else if (speed =3D=3D SPEED_10)
> > +             ctl &=3D ~BMCR_SPEED100;
> > +     else
> > +             dev_err(ds->dev, "unsupported speed: %d\n", speed);
> > +
> > +     if (duplex =3D=3D DUPLEX_FULL)
> > +             ctl |=3D BMCR_FULLDPLX;
> > +     else
> > +             ctl &=3D ~BMCR_FULLDPLX;
> > +
> > +     lan9303_phy_write(ds, port, MII_BMCR, ctl);
>=20
> There is no code here to program the resolved pause modes. Is it handled
> internally within the switch? (Please add a comment to this effect
> either in get_caps or here.)
>=20
> Thanks.
>=20

As I look into this, the part does have control flags for advertising
Symmetric and Asymmetric pause toward the link partner. The default is set
by a configuration strap on power-up. I am having trouble mapping the rx an=
d
tx pause parameters into symmetric and asymmetric controls. Where can I fin=
d
the proper definitions and mappings?

  ctl &=3D ~( ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_AYM);
  if(tx_pause)
    ctl |=3D ADVERTISE_PAUSE_CAP;
  if(rx_pause)
    ctl |=3D ADVERTISE_PAUSE_AYM;

If I can pause my transmissions (receive pause requests), then advertise
symmetric whether I ever sent pause requests or not.
If I can send pause requests (using flow control on my receive side), then =
make
sure asymmetric support is also advertised as rx_pause might have been clea=
r.
Not that if both rx and tx pause is supported, we can support either symmet=
ric
or asymmetric operating modes.

If I receive a pause request, it affects my transmit data flow. So one coul=
d
argue the rx_pause flag controls my ability to receive pause requests. I te=
nd
to overthink and almost always get these 50/50 propositions wrong.

Regards,
Jerry