Date:   Wed, 8 Mar 2023 15:46:42 +0200
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc:     Daniel Golle <daniel@makrotopia.org>, netdev@vger.kernel.org,
        linux-mediatek@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Mark Lee <Mark-MC.Lee@mediatek.com>,
        John Crispin <john@phrozen.org>, Felix Fietkau <nbd@nbd.name>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        DENG Qingfang <dqfext@gmail.com>,
        Landen Chao <Landen.Chao@mediatek.com>,
        Sean Wang <sean.wang@mediatek.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Vladimir Oltean <olteanv@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Jianhui Zhao <zhaojh329@gmail.com>,
        =?utf-8?B?QmrDuHJu?= Mork <bjorn@mork.no>,
        Frank Wunderlich <frank-w@public-files.de>,
        Alexander Couzens <lynxis@fe80.eu>
Subject: Re: [PATCH net-next v12 08/18] net: ethernet: mtk_eth_soc: fix
 1000Base-X and 2500Base-X modes
Message-ID: <20230308134642.cdxqw4lxtlgfsl4g@skbuf>
References: <cover.1678201958.git.daniel@makrotopia.org>
 <fd5c7ea79a7f84caac7d0b64b39fe5c4043edfa8.1678201958.git.daniel@makrotopia.org>
 <ZAhzDDjZ8+gxyo3V@shell.armlinux.org.uk>
 <ZAh7hA4JuJm1b2M6@makrotopia.org>
 <ZAiCh8wkdTBT+6Id@shell.armlinux.org.uk>
 <ZAiFOTRQI36nGo+w@makrotopia.org>
 <ZAiJqvzcUob2Aafq@shell.armlinux.org.uk>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZAiJqvzcUob2Aafq@shell.armlinux.org.uk>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Xd4RH9+57gpOYCM4UYN5A4jICU9bNSlL++FpMUPtBxag7W/5FTK/9OaYslnA?=
 =?us-ascii?Q?ainqsBVabEktcafAq0KXay1W8DUgjZLjuwkXmwWDn/0DHgbI+n45wGu+Rmsq?=
 =?us-ascii?Q?49MU5thgsJxC+qtBQvsgZQz4r4/jJ6xdViDwhU9tRKleqU6sLGlRqp1UOSm4?=
 =?us-ascii?Q?89yHSmNMCMILfcBxXK1U85OJxFf2pTRaSkCSmKNF/CL6JBG50xPTd+HqLSXu?=
 =?us-ascii?Q?UU6ICb879mhzZD2adXBVomnolV+FBjYKKZvQZmWWPEWP2cOSBBICr7l5FKGV?=
 =?us-ascii?Q?thox78APxVA/36nFJtQz0GBkXD+S8cRsW7hTVRw0Izbsmqr59hY/lenmCN+t?=
 =?us-ascii?Q?s4cpemCZAFJv4RUVnWsuvqktno6mQPaUDujjQbYQn9wf9ewg4W88nWk4SJfd?=
 =?us-ascii?Q?yNGXr06midVhPqkVD6zNHUxDgjdddYPImvLPWxTtlvBiLFXJB0FczJBcK7C0?=
 =?us-ascii?Q?tcLS6acrJSEjhK58NFmje8v50WftYzYatCooRAank+m9if5ObwCQTEh7Le8F?=
 =?us-ascii?Q?0BPBMt5VST96NXLKaikgMNPKC9Zja4+f1bekYgcovSh5CU+G/8ZPREyvDTsf?=
 =?us-ascii?Q?b99y+N0r3/LMVhCenLDrfuUBYpG9HYX0dOfCF5YqjqlOw80B/3ez9kTIRugD?=
 =?us-ascii?Q?ThjgBknanJy/hGVhZkrepui6KcrOzyaTqJ+rPgq8mYHPbcF9ReEvfrX2ZB+f?=
 =?us-ascii?Q?kvT27fAo6U3NG8W0XrOzob3ueGi6iEk5vxdrZ2gXACGclDlBK7EHmzBYR2Pq?=
 =?us-ascii?Q?/RQ4O8RcrrlhRujUt6dSyKE0CPRex6e5C8ivURCR/geWT31MZj2AiCeP9EWd?=
 =?us-ascii?Q?GmlrZbS5y/OSERRWLg+zOispt5guT6v7qmlneTlf6MIFjJBpzISU4L8sWGLq?=
 =?us-ascii?Q?hVn3EXESwmwltLKkx/LV+gpM1eQjg8ERAV4f1Ce3l1SBRMJEZUPHJlClayrB?=
 =?us-ascii?Q?asExNfitx2DugU2sKQ/E+zKSjRgfaTkfuZzDbTzMLZOE6Bv0xP9VvJ/muir6?=
 =?us-ascii?Q?nLQhFPZC5COrrMc7cx7zLRt0WRGM+Ov3ClXogAnBvN83Nwcl7bmt3bhJY7lX?=
 =?us-ascii?Q?izRVFsg2XMBLjMlLcyHcB3Kc2awu28IG/bE9liuNh8MPrIt1+quik6oiYn4G?=
 =?us-ascii?Q?iqAu/5VlCmSbwtuIidtSpb5ns3CKKImQ+hakGdf1MXCiENr8U4t6pYn9sKIA?=
 =?us-ascii?Q?1FE5y3iUrPujEF+EngaK0h2GN/zkm9ijgMvN1JGNjZKdlNLknVZF2aye7qQS?=
 =?us-ascii?Q?9EDTlBqXcPs1QuOnzrInI3F4F9iBrqja4mBFgQT/DsnIRLO3StN9wlwKs7xd?=
 =?us-ascii?Q?3yxQpxwQFrpKo28qr5xXXPsLpuqnJ9kjJtJ1VRRbVNQ21W/C90K5+AHf63uj?=
 =?us-ascii?Q?nMK6lObQBnVFMunWcyUj+/BAXJP4qBNTekuOdIzE7SNpq9RfpTjwrE8Mmpcp?=
 =?us-ascii?Q?qWbO7f7M5eNgUxF/H2PhGruBYg5j/BNGj0QL5s3sW0YJNde15hjrF/3Mhvu1?=
 =?us-ascii?Q?6rYxYMt0QOtKSIUmTCUj6nS7MhsG0s328K6XXLDt6J/Ug1TlhEKCN6BjjEvQ?=
 =?us-ascii?Q?zKfC4tm25XlpHnGMZusk8JC1maI5DXTnjew48UplN05V4vDtbu7bGON0wj5E?=
 =?us-ascii?Q?bQ=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d5b61cc8-bc44-43d2-30cd-08db1fdb8fb9
X-MS-Exchange-CrossTenant-AuthSource: AM0PR04MB6452.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Mar 2023 13:46:47.4509
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: HKsC2KXrh5NUz3NLYkyNAbg2cntCXM2rKrpO2IlmSRIZuAdS4B2H9M/qzmExOR2szlcFXAUffr0g5NJx+xW9pg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: GV1PR04MB9136
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, Mar 08, 2023 at 01:12:10PM +0000, Russell King (Oracle) wrote:
> So, what I would want to do is to move the decision about whether
> the PCS should enable in-band into the phylink core code instead
> of these random decisions being made in each PCS.
> 
> At that point, we can then make the decision about whether the
> ethtool autoneg bit should affect whether the PCS uses inband
> depending on whether the PCS is effectively the media facing
> entity, or whether there is a PHY attached - and if there is a PHY
> attached, ask the PHY whether it will be using in-band or not.
> 
> This also would give a way to ensure that all PCS adopt the same
> behaviour.
> 
> Does that sound a reasonable approach?
> 
> Strangely, I already have some patches along those lines in my
> net-queue branch. See:
> 
> net: phylink: add helpers for decoding mode
> net: use phylink_mode_*() helpers
> net: phylink: split PCS in-band from inband mode
> 
> It's nowhere near finished though, it was just an idea back in
> 2021 when the problem of getting rid of differing PCS behaviours
> was on my mind.

Having looked at those patches
(http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=a632167d226cf95d92cd887b2f1678e1539833b1)
and seen the way in which they are incomplete, could you sketch here how
do you see an actual pcs_validate() implementation making use of the new
"mode" argument?

I'd expect there to be some logic which changes the "mode", if the PCS
validation with the existing one fails... or?