Received-SPF: pass (google.com: domain of linux-kernel+bounces-121698-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Elad Nachman <enachman@marvell.com>
To: Andrew Lunn <andrew@lunn.ch>
CC: Taras Chornyi <taras.chornyi@plvision.eu>,
        "davem@davemloft.net"
	<davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "kory.maincent@bootlin.com" <kory.maincent@bootlin.com>,
        "thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
        "miquel.raynal@bootlin.com" <miquel.raynal@bootlin.com>,
        "przemyslaw.kitszel@intel.com" <przemyslaw.kitszel@intel.com>,
        "dkirjanov@suse.de" <dkirjanov@suse.de>,
        "netdev@vger.kernel.org"
	<netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [EXTERNAL] Re: [PATCH v2 0/5] Fix prestera driver fail to probe
 twice
Thread-Topic: [EXTERNAL] Re: [PATCH v2 0/5] Fix prestera driver fail to probe
 twice
Thread-Index: 
 AQHaeurjPmANQMrOHkqJdwxY/uh+37FBVLMAgAEesYCAACDkgIAD853QgACBRQCABNdBkA==
Date: Wed, 27 Mar 2024 17:27:41 +0000
Message-ID: 
 <BN9PR18MB4251B1533E14523AEADBA22FDB342@BN9PR18MB4251.namprd18.prod.outlook.com>
References: <20240320172008.2989693-1-enachman@marvell.com>
 <4104387a-d7b5-4029-b822-060ef478c6e3@lunn.ch>
 <BN9PR18MB42517F8E84C8C18078E45C37DB322@BN9PR18MB4251.namprd18.prod.outlook.com>
 <89a01616-57c2-4338-b469-695bdc731dee@lunn.ch>
 <BL1PR18MB42488523A5E05291EA57D0AEDB372@BL1PR18MB4248.namprd18.prod.outlook.com>
 <6dae31dc-8c4f-4b8d-80e4-120619119326@lunn.ch>
In-Reply-To: <6dae31dc-8c4f-4b8d-80e4-120619119326@lunn.ch>
Accept-Language: he-IL, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?iso-8859-1?Q?WqZMR/EGSQAGVGQiEq5CK+owlfJpS53FzQ3k+7osQC4r4myaRJ9PNthzIQ?=
 =?iso-8859-1?Q?I+A44+K1BM1otABHHKv9s95+e2HSFQT5TPfo8DxWgzH87R2l3mF57tF1VF?=
 =?iso-8859-1?Q?XBk2NAM8jCI6fM6XSTse3Izg2nQhJXv91jRsz0NcfPME+o1+tS1oDPNOp4?=
 =?iso-8859-1?Q?ECyeHbinOT1PQHxWbx1r08PmV02nFYUcueDVlljB+coTIgNONUajp7Nkr1?=
 =?iso-8859-1?Q?JxnvCIoeMRkNY0FEOPxqAWBn3t/WJ28dqLt12BRKy1EYognE6oMwRgJwvt?=
 =?iso-8859-1?Q?hOF7UBNJviYNEU5/rJ0kZElXn5K+XYj/M9Stie0oo4AN7ZDXBspacnGqi+?=
 =?iso-8859-1?Q?SCrOwPTzHgH7JQhvG11heXSHj/vcAQlNJorF+w3OJg6UP9llFNwuLwxy5Z?=
 =?iso-8859-1?Q?wFhOkczCEzEWbsqBDhTCEwq8edCLdavepIksD8ZvwZZYuCVZJJjCc/1tiy?=
 =?iso-8859-1?Q?Yt9cbx5UZzVnkMCfThbsViUzdalccMhzgbT+b5fX0AJPAC4PC4/n46eKKV?=
 =?iso-8859-1?Q?yWXTInSfzroHfLew6WnSV8x6PuskYzmVxhob+tQxlaA3ABMwH0pDCmJtCM?=
 =?iso-8859-1?Q?eP3Zn/6bQ944KGENGi5RQg5PyKgPJ7UfMolHlwsNaeyEPsc5uImrCAMEKN?=
 =?iso-8859-1?Q?tg5R+9dtJXqsj4jPSscIrntGbDMAjZXHcz06f9EOj4kIZgmxo05UPaYZ5h?=
 =?iso-8859-1?Q?eY8LgL4obDy7swcD2v9h58NGmAMunLN8K0gBQzvCjA1r2M9ihB0TUiJDYm?=
 =?iso-8859-1?Q?ZcYTswdqqi9716JjzJnxEKWng5a5jQdhWdsQzIwkyAZOMQuWS+Mf35ouq+?=
 =?iso-8859-1?Q?MsdAL7IS95StPko+hjPc/WfKvvDtEZWS5qjj+vsuDCNaav4OvQ7/vBHAEj?=
 =?iso-8859-1?Q?9D8lG08jIfU4L+Anu7UCB1Icnffy1j55laNfwXAtR6Tod7LYH98UAZtrRU?=
 =?iso-8859-1?Q?B4wN/7//bAGd9Vidu4+4t3gAu2RX99nVwKzeFHaeY5EbS2dE2eG1Vf27ff?=
 =?iso-8859-1?Q?JqovnEnR7w5hK/OAi4hhHfCwY4BGPivmUa6kt9mShu+eO1Hn+vOefRnpyJ?=
 =?iso-8859-1?Q?tP+vfZnnPGHTXxr9D++Y037XLNEeYwXxF2+6Ma7J/hUfeaQMPHInOCDmEp?=
 =?iso-8859-1?Q?MfKA6zzoNOfOJIG7mrbkH2VF21fDb2HPVUt2Q4AOammXbj5/0Gz/455FfM?=
 =?iso-8859-1?Q?TrYaopZ/cWUHLKY+7UkR7MYNwW1Wj0OMQHzzKaOngfUuNKthZBUqjVCkHs?=
 =?iso-8859-1?Q?msD2sbJs0dkbj9cv7g6Heb+KeS/rZq8g1tpiToNltBEASA7l0wH8hYb/lB?=
 =?iso-8859-1?Q?p8z1Vol/DcNWSsVFfluy8WXDQyvmp8nyqp/ejmvFyvdpHY/9b37hdhDWVs?=
 =?iso-8859-1?Q?LMwlR/97HlJCjnQgMIzhvbchJscAKAae+H2M1TbLKOVjZHnK9jerAZ+RD9?=
 =?iso-8859-1?Q?V/GZ0rY7kOFvJ+ygJXLn5v56orwpm4bgF8I76rOq8Ota2Pqp2VriCH0/yO?=
 =?iso-8859-1?Q?owF+nIUPq2W6Tm70mvg67ADZh3kjaqE/ek0tJh7uBdPEhB/H+J5znhLwHL?=
 =?iso-8859-1?Q?qwKw9AfSdKuOgIf2Ek2XkfiomWgGMIkjS5D4NOPa3OTMXj8i4g7af6qgyt?=
 =?iso-8859-1?Q?nzWxyyDyupsZYZHghY/kSMrV5BX/ASU9ib?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR18MB4251.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d9ef1118-f7f7-4e7d-d231-08dc4e8334e4
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Mar 2024 17:27:41.4290
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 5y1vIH08GKNEx2mQUHUbymchkGS4B+uIGg5e9qXSrnzdIc2YPEyJZMGpdxkAx1nivCHRcd3UG9cFd+2QeVkW4w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV8PR18MB5855
X-Proofpoint-ORIG-GUID: MX057xmhy6dEYHOlHIXAtskouQcmLRKL
X-Proofpoint-GUID: MX057xmhy6dEYHOlHIXAtskouQcmLRKL
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-03-27_14,2024-03-27_01,2023-05-22_02

Hi Andrew,

We have made internal technical review of the issues you have raised (retur=
n version API, try to get version API before starting to initialize and loa=
d the firmware, clear configuration API) versus the delay saved (almost 30 =
seconds minus several seconds to perform and complete the API calls) - arou=
nd 20 seconds or so.

Existing customers we have talked to seem to be able to cope with the exist=
ing delay.

Unfortunately, the amount of coding and testing involved with saving these =
20 seconds or so is beyond our available development manpower at this speci=
fic point in time.

Unfortunately, we will have to defer making the development you have reques=
ted to a later period in time.

Elad.


> -----Original Message-----
> From: Andrew Lunn <andrew@lunn.ch>
> Sent: Sunday, March 24, 2024 5:25 PM
> To: Elad Nachman <enachman@marvell.com>
> Cc: Taras Chornyi <taras.chornyi@plvision.eu>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> kory.maincent@bootlin.com; thomas.petazzoni@bootlin.com;
> miquel.raynal@bootlin.com; przemyslaw.kitszel@intel.com;
> dkirjanov@suse.de; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [EXTERNAL] Re: [PATCH v2 0/5] Fix prestera driver fail to pr=
obe
> twice
>=20
> > > > Originally, the pain point for Kory was the rmmod + insmod
> > > > re-probing failure, Which is only fixed by the first two commits,
> > > > so I see little point in submitting 3-5 alone, Without fixing Kory'=
s
> problem.
> > >
> > > I thought Kory's problem was actually EPROBE_DEFER? The resources
> > > needed for the PoE are not available, so probing the switch needs to
> > > happen again later, when PoE can get the resources it needs.
> >
> > No, the PoE is the general high level application where he noted the
> problem.
> > There is no PoE code nor special PoE resources in the Prestera driver.
>=20
> So here is K=F6ry email:
>=20
> https://urldefense.proofpoint.com/v2/url?u=3Dhttps-
> 3A__lore.kernel.org_netdev_20240208101005.29e8c7f3-40kmaincent-2DXPS-
> 2D13-2D7390_T_-
> 23mb898bb2a4bf07776d79f1a19b6a8420716ecb4a3&d=3DDwIDAw&c=3DnKjWec2
> b6R0mOyPaz7xtfQ&r=3DeTeNTLEK5-
> TxXczjOcKPhANIFtlB9pP4lq9qhdlFrwQ&m=3DSD1MhKC11sFmp4Q8l76N_DgGdac
> 4aMCTdPsa7Pofb73HEqAGtJ-1p0-
> etIyyldC7&s=3DVWat9LPub52H3nUez4itmkpuMipnYD3Ngn-paFC9wd4&e=3D
>=20
> I don't see why the prestera needs to be involved in PoE itself. It is ju=
st a MAC.
> PoE happens much lower down in the network stack. Same as Prestera uses
> phylink, it does not need to know about the PHYs or the SFP modules, phyl=
ink
> manages them, not prestera.
>=20
> > The problem was caused because the module exit was lacking the so
> > called "switch HW reset" API call which would cause the firmware to
> > exit to the firmware loader on the firmware CPU, and move to the state
> > in the state machine when it can receive new firmware from the host
> > CPU (running the Prestera switchDev driver).
> >
> > >
> > > But if that is going to take 30 seconds, i'm not sure we can call
> > > EPROBE_DEFER solved.
> > >
> > > The later patches are pretty simple, don't need discussion, so could
> > > be merged. However, i think we need to explore different possible
> > > solutions for firmware {re}loading.
> > >
> > > > The problem is not with the hardware, but with the existing
> > > > firmware code on the Firmware cpu, most probably secure-boot
> > > > protected, which lacks the ABIs to report to The kernel what is
> > > > loaded, what version, what
> > > state, etc.
> > >
> > > Can you at least tell if it is running firmware?
> >
> > There is no existing API/ABI for that.
>=20
> Do you at least have the ability to determine if an API call exists or no=
t? It
> sounds like your firmware needs extending to support returning the versio=
n.
> If the API is missing, you know it is 4.1 or older. If it does exist, it =
will return
> 4.2 or higher.
>=20
> > > Can you explain the boot in a bit more detail. Are you saying it
> > > could be running an old firmware when the driver first loads? So you
> > > need to hit it with
> >
> > Exactly.
> >
> > > a reset in order to load the firmware for /lib/firmware, which might
> > > be newer than what it is already running?
> >
> > Right. And there is also the configuration. There is no telling what
> > kind of Configuration the existing firmware is running. Just using the
> > existing firmware Will lead to the situation where Linux kernel side
> > will report certain configuration (via ip link / ip addr / tc , etc.) b=
ut the
> firmware configuration is completely different.
>=20
> Well, during probe and -EPRODE_DEFER, linux has no configuration, since t=
he
> driver failed to probe. However, for a rmmod/modprobe, the firmware could
> have stale configuration. However pretty much every device i've come acro=
ss
> has the concept of a software reset which clears out the configuration. S=
eems
> to be something else your firmware is missing.
>=20
> 	Andrew