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

[Yibo Dong]

On Fri, Aug 15, 2025 at 03:36:10PM +0200, Andrew Lunn wrote:
> 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
> 

Ok, I will fill 0xffffffff in mucse_mbx_ifinsmod to echo firmware.

Thanks for your feedback.