[RFC PATCH v2 4/6] ath79: support for TP-Link EAP245 v1

[Sander Vanheule]

Hi Adrian,

Thank you for taking your time to provide feedback.


On Mon, 2020-07-20 at 22:54 +0200, mail at adrianschmutzler.de wrote:
> > > Tested on the EAP245 v1 running the latest firmware (v1.4.0). The
> > > binary patch might not apply to uclited from other firmware
> > > versions.
> > > 
> > > Signed-off-by: Sander Vanheule <sander at svanheule.net>
> > 
> > Seems like I was overdue on a proper read of the kernel patch
> > submission
> > guidelines. My understanding from the guidelines and your previous
> > mail [1],
> > is that these lines aren't about the literal patch contents per se,
> > but also
> > about the intention of the patch and the provided functionality.
> > 
> > So the fact that the bulk of the EAP245 v1's DTS was moved to the
> > 1- port
> > DTSI, shouldn't be an issue to attribute device support to Julien
> > in this patch,
> > right?
> 
> I see that differently. For me, providing device support for a device
> A and using similar code for a bunch of devices B to D is a different
> patch.
> 
> I don't think a Signed-off-by is correct here, as Julien is _not_ an
> author of your patch, as he intended to provide support for the
> EAP245 and not for the 1-port EAP2x5 devices.
> > Would you consider the following appropriate for this patch?
> > 
> >    EAP245 v1 support originally implemented by Julien Dusser.
> 
> That's nice but irrelevant without proper explanation ("why is EAP245
> relevant at all").

This patch (4/6) specifically enables support for the EAP245 v1, the
device Julien worked on. You can see a history of his and my changes on
the EAP245v1-only DTS on Julien's GitHub page:
https://github.com/j-d-r/openwrt/commits/master-eap245-original-u-boot/target/linux/ath79/dts/qca9563_tplink_eap245-v1.dts

> 
> If you really want to refer to that prior work, IMO a proper solution
> would be to just add something like "Implementation of these devices
> is based on the prior work of XY supporting device YZ in commit
> xxxxxxxxx."
> 
> Then, everybody can look up what XY has done and will see the proper
> authorship in the reference.
> >    SoC MDIO integration, factory flashing method, and final patch
> > by
> >    Sander Vanheule.
> > 
> >    Co-developed-by: Julien Dusser <julien.dusser at free.fr>
> >    Signed-of-By: Julien Dusser <julien.dusser at free.fr>
> 
> The initial author needs no Co-developed-by, as he is mentioned in
> the From field.
> From/Co-developed-by is about authorship, Signed-off-by is about
> legal accountability.
> 
> The latter is one reason why you technically actually can only add
> Juliens Signed-off-by if this patch is combined submission of both of
> you, where both people have actually checked the final patch for
> correctness. If that's not the case, it's not Co-developed-by, but
> Julien would be the author, and you would have to note every single
> change before your Signed-off-by to make obvious which parts are
> covered by his SoB and what has been changed since then and thus is
> covered by your SoB.
> (example for the latter may be found here: 
> https://github.com/openwrt/openwrt/commit/ed087cba8a8e41f76f9487caa34eff926ea8a065
> )
> 
> Since this appears to me to be "your" patch, and not a submission by
> both of you, for me it would be more correct to just have your
> SoB/From: only.
> If the original patch was mine, I'd actually be quite mad at you if
> you used my Signed-off-by for a different submission.
> 

You are right in that Julien did not formally sign of on this specific
patch. I seemed to remember I asked him whether I should include a
Signed-off-by for him, but it turns out that was for another patch. Due
to my lacking understanding of the implications of a Signed-off-by at
the time, I must have misremembered.

So for this patch and 3/6 (the DTSI), I will only sign off for myself.
(And I think I owe Julien an apology for trying to formally attach his
name to this patch.)


Best,
Sander