Re: [PATCH v4 2/5] net: rnpgbe: Add n500/n210 chip support

[Andrew Lunn <andrew@lunn.ch>]

On Fri, Aug 15, 2025 at 03:21:03PM +0800, Yibo Dong wrote:
> On Fri, Aug 15, 2025 at 05:56:30AM +0200, Andrew Lunn wrote:
> > > It means driver version 0.2.4.16.
> > 
> > And what does that mean?
> > 
> > > I used it in 'mucse_mbx_ifinsmod'(patch4, I will move this to that patch),
> > > to echo 'driver version' to FW. FW reply different command for different driver.
> > 
> > There only is one driver. This driver.
> > 
> > This all sounds backwards around. Normally the driver asks the
> > firmware what version it is. From that, it knows what operations the
> > firmware supports, and hence what it can offer to user space.
> > 
> > So what is your long terms plan? How do you keep backwards
> > compatibility between the driver and the firmware?
> > 
> > 	Andrew
> > 
> 
> To the driver, it is the only driver. It get the fw version and do
> interactive with fw, this is ok.
> But to the fw, I think it is not interactive with only 'this driver'?
> Chips has been provided to various customers with different driver
> version......

They theoretically exist, but mainline does not care about them. 

> More specific, our FW can report link state with 2 version:
> a: without pause status (to driver < 0.2.1.0)
> b: with pause status (driver >= 0.2.1.0)

But mainline does not care about this. It should ask the firmware, do
you support pause? If yes, report it, if not EOPNOTSUP. You want to be
able to run any version of mainline on any version of the
firmware. This means the ABI between the driver and the firmware is
fixed. You can extend the ABI, but you cannot make fundamental
changes, like adding new fields in the middle of messages. With care,
you can add new fields to the end of an existing messages, but you
need to do it such that you don't break older versions of the driver
which don't expect it.

Please look at other drivers. This is how they all do this. I don't
know of any driver which reports its version to the firmware and
expects the firmware to change its ABI.

So maybe you should just fill this version with 0xffffffff so the
firmware enables everything, and that is the ABI you use. Does the
firmware have an RPC to get its version? You can then use that for
future extensions to the ABI. Same as all other drivers.

	Andrew