Re: [git:v4l-dvb/for_v3.3] [media] DVB: dib0700, add support for Nova-TD LEDs

Re: [git:v4l-dvb/for_v3.3] [media] DVB: dib0700, add support for Nova-TD LEDs

[Michael Krufky]

Mauro,

Why was my sign-off changed to an Ack?

As you can see, I worked *with* Jiri to help him create this patchset.

During review, I noticed a poorly named function, which I renamed before 
pusging it into my own tree.  Patrick saw this, and merged my changes 
into into his tree.

Why did I go through this effort to help another developer add value to 
one of our drivers, and additional effort to make a small cleanup, push 
the changes into my own tree and issue a pull request?  I was thanked by 
Patrick.  Everybody's signature is on the patch, but you then go and 
remove my signature, and add a forged "ack"?  I don't understand this, 
Mauro.

Why didn't you just take my pull request?  Instead, you have changed my 
signature?  What is the point of a signature if it will become mangled?

Please don't do this.  Or, if you have *some* legitimate reason to 
change my signature, you should at LEAST check with me before committing 
it into kernel history.

-Mike

On 01/17/2012 08:02 PM, Mauro Carvalho Chehab wrote:
> This is an automatic generated email to let you know that the following patch were queued at the
> http://git.linuxtv.org/media_tree.git tree:
>
> Subject: [media] DVB: dib0700, add support for Nova-TD LEDs
> Author:  Jiri Slaby<jslaby@suse.cz>
> Date:    Tue Jan 10 14:11:25 2012 -0300
>
> Add an override of read_status to intercept lock status. This allows
> us to switch LEDs appropriately on and off with signal un/locked.
>
> The second phase is to override sleep to properly turn off both.
>
> This is a hackish way to achieve that.
>
> Thanks to Mike Krufky for his help.
>
> Signed-off-by: Jiri Slaby<jslaby@suse.cz>
> Acked-by: Michael Krufky<mkrufky@linuxtv.org>
> Signed-off-by: Patrick Boettcher<pboettcher@kernellabs.com>
> Signed-off-by: Mauro Carvalho Chehab<mchehab@redhat.com>
>
>   drivers/media/dvb/dvb-usb/dib0700.h         |    2 +
>   drivers/media/dvb/dvb-usb/dib0700_devices.c |   41 ++++++++++++++++++++++++++-
>   2 files changed, 42 insertions(+), 1 deletions(-)
>
> ---
>
> http://git.linuxtv.org/media_tree.git?a=commitdiff;h=d43272a4e898a1e43e5ac351ab625b7a40b39e88
>
> diff --git a/drivers/media/dvb/dvb-usb/dib0700.h b/drivers/media/dvb/dvb-usb/dib0700.h
> index 9bd6d51..7de125c 100644
> --- a/drivers/media/dvb/dvb-usb/dib0700.h
> +++ b/drivers/media/dvb/dvb-usb/dib0700.h
> @@ -48,6 +48,8 @@ struct dib0700_state {
>   	u8 disable_streaming_master_mode;
>   	u32 fw_version;
>   	u32 nb_packet_buffer_size;
> +	int (*read_status)(struct dvb_frontend *, fe_status_t *);
> +	int (*sleep)(struct dvb_frontend* fe);
>   	u8 buf[255];
>   };
>
> diff --git a/drivers/media/dvb/dvb-usb/dib0700_devices.c b/drivers/media/dvb/dvb-usb/dib0700_devices.c
> index 3ab45ae..f9e966a 100644
> --- a/drivers/media/dvb/dvb-usb/dib0700_devices.c
> +++ b/drivers/media/dvb/dvb-usb/dib0700_devices.c
> @@ -3105,6 +3105,35 @@ static int stk7070pd_frontend_attach1(struct dvb_usb_adapter *adap)
>   	return adap->fe_adap[0].fe == NULL ? -ENODEV : 0;
>   }
>
> +static int novatd_read_status_override(struct dvb_frontend *fe,
> +		fe_status_t *stat)
> +{
> +	struct dvb_usb_adapter *adap = fe->dvb->priv;
> +	struct dvb_usb_device *dev = adap->dev;
> +	struct dib0700_state *state = dev->priv;
> +	int ret;
> +
> +	ret = state->read_status(fe, stat);
> +
> +	if (!ret)
> +		dib0700_set_gpio(dev, adap->id == 0 ? GPIO1 : GPIO0, GPIO_OUT,
> +				!!(*stat&  FE_HAS_LOCK));
> +
> +	return ret;
> +}
> +
> +static int novatd_sleep_override(struct dvb_frontend* fe)
> +{
> +	struct dvb_usb_adapter *adap = fe->dvb->priv;
> +	struct dvb_usb_device *dev = adap->dev;
> +	struct dib0700_state *state = dev->priv;
> +
> +	/* turn off LED */
> +	dib0700_set_gpio(dev, adap->id == 0 ? GPIO1 : GPIO0, GPIO_OUT, 0);
> +
> +	return state->sleep(fe);
> +}
> +
>   /**
>    * novatd_frontend_attach - Nova-TD specific attach
>    *
> @@ -3114,6 +3143,7 @@ static int stk7070pd_frontend_attach1(struct dvb_usb_adapter *adap)
>   static int novatd_frontend_attach(struct dvb_usb_adapter *adap)
>   {
>   	struct dvb_usb_device *dev = adap->dev;
> +	struct dib0700_state *st = dev->priv;
>
>   	if (adap->id == 0) {
>   		stk7070pd_init(dev);
> @@ -3134,7 +3164,16 @@ static int novatd_frontend_attach(struct dvb_usb_adapter *adap)
>   	adap->fe_adap[0].fe = dvb_attach(dib7000p_attach,&dev->i2c_adap,
>   			adap->id == 0 ? 0x80 : 0x82,
>   			&stk7070pd_dib7000p_config[adap->id]);
> -	return adap->fe_adap[0].fe == NULL ? -ENODEV : 0;
> +
> +	if (adap->fe_adap[0].fe == NULL)
> +		return -ENODEV;
> +
> +	st->read_status = adap->fe_adap[0].fe->ops.read_status;
> +	adap->fe_adap[0].fe->ops.read_status = novatd_read_status_override;
> +	st->sleep = adap->fe_adap[0].fe->ops.sleep;
> +	adap->fe_adap[0].fe->ops.sleep = novatd_sleep_override;
> +
> +	return 0;
>   }
>
>   /* S5H1411 */

Re: [git:v4l-dvb/for_v3.3] [media] DVB: dib0700, add support for Nova-TD LEDs

[Patrick Boettcher]

On Wednesday 18 January 2012 14:18:48 Michael Krufky wrote:
> Mauro,
> 
> Why was my sign-off changed to an Ack?
> 
> As you can see, I worked *with* Jiri to help him create this
> patchset.
> 
> During review, I noticed a poorly named function, which I renamed
> before pusging it into my own tree.  Patrick saw this, and merged my
> changes into into his tree.
> 
> Why did I go through this effort to help another developer add value
> to one of our drivers, and additional effort to make a small
> cleanup, push the changes into my own tree and issue a pull request?
>  I was thanked by Patrick.  Everybody's signature is on the patch,
> but you then go and remove my signature, and add a forged "ack"?  I
> don't understand this, Mauro.

I think it is my fault.

I haven't merged your tree but I merged Jiri's patches as is. (git am)  
I completely oversaw your pull request and issued mine. 

Mauro in IRC told me that you issued a PULL request as well. Not being 
aware that you have made any modifications Mauro suggest to pull from me 
and add an Ack-By to the patches.

So he did not remove anything but trusted me too much.

--
Patrick Boettcher

Kernel Labs Inc.
http://www.kernellabs.com/

Re: [git:v4l-dvb/for_v3.3] [media] DVB: dib0700, add support for Nova-TD LEDs

[Michael Krufky]

On 01/18/2012 08:54 AM, Patrick Boettcher wrote:
> On Wednesday 18 January 2012 14:18:48 Michael Krufky wrote:
>> Mauro,
>>
>> Why was my sign-off changed to an Ack?
>>
>> As you can see, I worked *with* Jiri to help him create this
>> patchset.
>>
>> During review, I noticed a poorly named function, which I renamed
>> before pusging it into my own tree.  Patrick saw this, and merged my
>> changes into into his tree.
>>
>> Why did I go through this effort to help another developer add value
>> to one of our drivers, and additional effort to make a small
>> cleanup, push the changes into my own tree and issue a pull request?
>>   I was thanked by Patrick.  Everybody's signature is on the patch,
>> but you then go and remove my signature, and add a forged "ack"?  I
>> don't understand this, Mauro.
>
> I think it is my fault.
>
> I haven't merged your tree but I merged Jiri's patches as is. (git am)
> I completely oversaw your pull request and issued mine.
>
> Mauro in IRC told me that you issued a PULL request as well. Not being
> aware that you have made any modifications Mauro suggest to pull from me
> and add an Ack-By to the patches.
>
> So he did not remove anything but trusted me too much.

Hmmmm...  That's unfortunate.  I also made various corrections in the 
commit messages.

I *meant* this to go to Mauro, I didn't realize it was going to 
linux-media list.  Now I see that there was a reply-to header set, so 
that explains why "reply" had larger effects ;-)

Either way, Mauro, Can you remove those patches and re-apply them from 
my tree?  Please feel free to add Patrick's Sign-off.

For the record, I actually worked *with* Jiri on these patches -- I told 
him how to toggle the LEDs, I told him which functions to override, and 
I showed him examples of how it was done elsewhere within dib0700 as 
well as how other drivers do it, and I encouraged him to submit these 
patches because I am trying to get more people involved in DVB development.

I just feel violated to have my signature stripped away from something 
that I took part in, and then to see additional signatures added on top 
made matters worse.

-Mike

Re: [git:v4l-dvb/for_v3.3] [media] DVB: dib0700, add support for Nova-TD LEDs

[Mauro Carvalho Chehab]

Em 18-01-2012 12:38, Michael Krufky escreveu:
> On 01/18/2012 08:54 AM, Patrick Boettcher wrote:
>> On Wednesday 18 January 2012 14:18:48 Michael Krufky wrote:
>>> Mauro,
>>>
>>> Why was my sign-off changed to an Ack?
>>>
>>> As you can see, I worked *with* Jiri to help him create this
>>> patchset.
>>>
>>> During review, I noticed a poorly named function, which I renamed
>>> before pusging it into my own tree.  Patrick saw this, and merged my
>>> changes into into his tree.
>>>
>>> Why did I go through this effort to help another developer add value
>>> to one of our drivers, and additional effort to make a small
>>> cleanup, push the changes into my own tree and issue a pull request?
>>>   I was thanked by Patrick.  Everybody's signature is on the patch,
>>> but you then go and remove my signature, and add a forged "ack"?  I
>>> don't understand this, Mauro.
>>
>> I think it is my fault.
>>
>> I haven't merged your tree but I merged Jiri's patches as is. (git am)
>> I completely oversaw your pull request and issued mine.
>>
>> Mauro in IRC told me that you issued a PULL request as well. Not being
>> aware that you have made any modifications Mauro suggest to pull from me
>> and add an Ack-By to the patches.
>>
>> So he did not remove anything but trusted me too much.
> 
> Hmmmm...  That's unfortunate.  I also made various corrections in the commit messages.
> 
> I *meant* this to go to Mauro, I didn't realize it was going to linux-media list. 
> Now I see that there was a reply-to header set, so that explains why "reply" had larger effects ;-)

With turned to be a good thing, as Patrick could reply on it ;)

> Either way, Mauro, Can you remove those patches and re-apply them from my tree?  Please feel free to add Patrick's Sign-off.

Sorry, but these would cause two very bad side effects:

1) I would need to rebase the tree to actually remove them, or to mangle the
history;

2) I'm planning to send today the last patches for the merge window. Those patches
are already at -next, together with a bunch of other patches. Removing them would
mean that I would need to wait for another day, probably missing the merge window.
> 
> For the record, I actually worked *with* Jiri on these patches -- I told him 
> how to toggle the LEDs, I told him which functions to override, and I showed him
> examples of how it was done elsewhere within dib0700 as well as how other drivers do it,
> and I encouraged him to submit these patches because I am trying to get more people 
> involved in DVB development.

Recorded.

For the record, SOB is the "Developer's Certificate of Origin" [1], e. g. it is used to
track who has manipulated the patch until it achieves upstream. It doesn't 
identify who authored the patch [2]. The main author of the patch is identified 
at the "From:". The other authors are identified inside the patch description,
as there's no special meta-data for co-authors.

Also for the record, when someone makes a patch for a maintained driver, I wait
for a few days in order to see if the maintainer will apply it. If he applies, 
I just pick from his tree. If the patch is trivial (new board addition, typo fixes),
I may just apply it, if I know the maintainer won't complain (several maintainers
seem to prefer this workflow, as they don't care enough to comment trivial patches).

So, if you wanted your SOB on the patches, you should either do:

	1) send it to Jiri for him to add them at the time the patches got posted;
	2) Submit the patches yourself, with both SOB's.

> I just feel violated to have my signature stripped away from something that I took part in,
> and then to see additional signatures added on top made matters worse.

Your signature wasn't stripped away. It was simply not there at the patches:
	http://patchwork.linuxtv.org/patch/9429/
	http://patchwork.linuxtv.org/patch/9426/
	http://patchwork.linuxtv.org/patch/9427/
	http://patchwork.linuxtv.org/patch/9428/

As Patrick picked the patches from the ML, the patch flow was:

Jiri -> Patrick -> me.

That means 3 SOB's on it, in order to represent such patch flow.

Regards,
Mauro

[1] http://linuxtv.org/wiki/index.php/Development:_Submitting_Patches#Developer.27s_Certificate_of_Origin_1.1

[2] As a general rule, the author gets the first SOB. Yet, there are
cases where someone's else takes the responsibility for submitting 
that patch upstream.
> 
> -Mike
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html