Re: [PATCH net-next RFC 2/5] r8152: update to version two

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Nov 09, 2022 at 03:50:59PM +1100, Albert Zhou wrote:
> On 9/11/22 04:28, Greg KH wrote:
> > >   // SPDX-License-Identifier: GPL-2.0-only
> > >   /*
> > > - *  Copyright (c) 2014 Realtek Semiconductor Corp. All rights reserved.
> > > + *  Copyright (c) 2021 Realtek Semiconductor Corp. All rights reserved.
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU General Public License
> > > + * version 2 as published by the Free Software Foundation.
> > To start with, this is not correct.  Don't add back license boiler-plate
> > code.
> 
> Hi Greg,
> 
> My apologies, I was unaware of this. This can be easily removed.
> 
> > 
> > And you just changed the copyright notice incorrectly, that is not ok.
> > 
> 
> When I replaced the version-one code with the version-two code, I assumed
> the authors' copyright would be correct. What is the correct copyright
> notice?

The correct way would be to list all years that the copyright was
asserted for the file.  Your patch removed the copyright notice for an
older year, which isn't ok.

But the larger issue here is that just wholesale replacing the in-tree
driver with an out-of-tree one isn't going to work.  As others have
pointed out, you need to break the changes up into
one-patch-per-logical-change and drag the driver forward that way.

The easiest way for you to do this is to clean up the out-of-tree driver
on its own, removing all the backwards compatibility stuff, and then try
to figure out what features are different and add them to the in-kernel
driver, one by one.

It's not an easy task, but as you have the hardware to test with, should
be doable.

good luck!

> > > + *
> > > + *  This product is covered by one or more of the following patents:
> > > + *  US6,570,884, US6,115,776, and US6,327,625.
> > Oh wow.  That's playing with fire...
> 
> Do you believe this prohibits the code from being in the kernel?

No I do not.  It's just not something that is normally advertised in the
kernel for obvious reasons :)

thanks,

greg k-h