diff mbox series

tpm: remove tpm_dev_wq_lock

Message ID 20190211105835.16851-1-bigeasy@linutronix.de (mailing list archive)
State New, archived
Headers show
Series tpm: remove tpm_dev_wq_lock | expand

Commit Message

Sebastian Andrzej Siewior Feb. 11, 2019, 10:58 a.m. UTC
Added in commit

  9e1b74a63f776 ("tpm: add support for nonblocking operation")

but never actually used it.

Cc: Philip Tricca <philip.b.tricca@intel.com>
Cc: Tadeusz Struk <tadeusz.struk@intel.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/char/tpm/tpm-dev-common.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Jarkko Sakkinen Feb. 11, 2019, 2:11 p.m. UTC | #1
On Mon, Feb 11, 2019 at 11:58:35AM +0100, Sebastian Andrzej Siewior wrote:
> Added in commit
> 
>   9e1b74a63f776 ("tpm: add support for nonblocking operation")
> 
> but never actually used it.
> 
> Cc: Philip Tricca <philip.b.tricca@intel.com>
> Cc: Tadeusz Struk <tadeusz.struk@intel.com>
> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

You should use Fixes-tag e.g.

Fixes: <12 first chars from SHA-1> ("<short summary>")

/Jarkko
Tadeusz Struk Feb. 12, 2019, 1:45 a.m. UTC | #2
On 2/11/19 2:58 AM, Sebastian Andrzej Siewior wrote:
> Added in commit
> 
>   9e1b74a63f776 ("tpm: add support for nonblocking operation")
> 
> but never actually used it.

It was used in one of the early versions of this patch
https://patchwork.kernel.org/patch/10559151/

Not needed later. 

Acked-by: Tadeusz Struk <tadeusz.struk@intel.com>
Thanks,
Sebastian Andrzej Siewior Oct. 10, 2019, 4:03 p.m. UTC | #3
On 2019-02-11 16:11:45 [+0200], Jarkko Sakkinen wrote:
> On Mon, Feb 11, 2019 at 11:58:35AM +0100, Sebastian Andrzej Siewior wrote:
> > Added in commit
> > 
> >   9e1b74a63f776 ("tpm: add support for nonblocking operation")
> > 
> > but never actually used it.
> > 
> > Cc: Philip Tricca <philip.b.tricca@intel.com>
> > Cc: Tadeusz Struk <tadeusz.struk@intel.com>
> > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> You should use Fixes-tag e.g.
> 
> Fixes: <12 first chars from SHA-1> ("<short summary>")

Is this the only reason why it has not been picked up? A fixes line
which triggers stable backports for something that does need to be
backported?

> /Jarkko

Sebastian
Jarkko Sakkinen Oct. 14, 2019, 7:39 p.m. UTC | #4
On Thu, Oct 10, 2019 at 06:03:13PM +0200, Sebastian Andrzej Siewior wrote:
> On 2019-02-11 16:11:45 [+0200], Jarkko Sakkinen wrote:
> > On Mon, Feb 11, 2019 at 11:58:35AM +0100, Sebastian Andrzej Siewior wrote:
> > > Added in commit
> > > 
> > >   9e1b74a63f776 ("tpm: add support for nonblocking operation")
> > > 
> > > but never actually used it.
> > > 
> > > Cc: Philip Tricca <philip.b.tricca@intel.com>
> > > Cc: Tadeusz Struk <tadeusz.struk@intel.com>
> > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > 
> > You should use Fixes-tag e.g.
> > 
> > Fixes: <12 first chars from SHA-1> ("<short summary>")
> 
> Is this the only reason why it has not been picked up? A fixes line
> which triggers stable backports for something that does need to be
> backported?

Fully agree with you. Frankly, I don't really remember anymore why I
responded that way. My guess is that I responded that to a worng patch.

Please just ping immediatelly. Sometimes when dealing with dozens of
patches this kind of human error might happen.

In any case, the patch is applied.

/Jarkko
Jarkko Sakkinen Oct. 28, 2019, 8:24 p.m. UTC | #5
On Mon, Oct 14, 2019 at 10:39:42PM +0300, Jarkko Sakkinen wrote:
> On Thu, Oct 10, 2019 at 06:03:13PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2019-02-11 16:11:45 [+0200], Jarkko Sakkinen wrote:
> > > On Mon, Feb 11, 2019 at 11:58:35AM +0100, Sebastian Andrzej Siewior wrote:
> > > > Added in commit
> > > > 
> > > >   9e1b74a63f776 ("tpm: add support for nonblocking operation")
> > > > 
> > > > but never actually used it.
> > > > 
> > > > Cc: Philip Tricca <philip.b.tricca@intel.com>
> > > > Cc: Tadeusz Struk <tadeusz.struk@intel.com>
> > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > 
> > > You should use Fixes-tag e.g.
> > > 
> > > Fixes: <12 first chars from SHA-1> ("<short summary>")
> > 
> > Is this the only reason why it has not been picked up? A fixes line
> > which triggers stable backports for something that does need to be
> > backported?
> 
> Fully agree with you. Frankly, I don't really remember anymore why I
> responded that way. My guess is that I responded that to a worng patch.
> 
> Please just ping immediatelly. Sometimes when dealing with dozens of
> patches this kind of human error might happen.
> 
> In any case, the patch is applied.

OK, so. Gave a relook at this:

This gives checkpatch.pl error:

0012-tpm-remove-tpm_dev_wq_lock.patch
-------------------------------------
ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 9e1b74a63f77 ("tpm: add support for nonblocking operation")'
#8: 
  9e1b74a63f776 ("tpm: add support for nonblocking operation")

total: 1 errors, 0 warnings, 7 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Please send me a new patch with a legit fixes line. It is a fix to
regression even if it is a cosmetic one.

I'll drop the current patch from my tree and apply a new one once
I get it from you.

Thanks.

/Jarkko
Jarkko Sakkinen Oct. 28, 2019, 8:26 p.m. UTC | #6
On Mon, Oct 28, 2019 at 10:24:19PM +0200, Jarkko Sakkinen wrote:
> On Mon, Oct 14, 2019 at 10:39:42PM +0300, Jarkko Sakkinen wrote:
> > On Thu, Oct 10, 2019 at 06:03:13PM +0200, Sebastian Andrzej Siewior wrote:
> > > On 2019-02-11 16:11:45 [+0200], Jarkko Sakkinen wrote:
> > > > On Mon, Feb 11, 2019 at 11:58:35AM +0100, Sebastian Andrzej Siewior wrote:
> > > > > Added in commit
> > > > > 
> > > > >   9e1b74a63f776 ("tpm: add support for nonblocking operation")
> > > > > 
> > > > > but never actually used it.
> > > > > 
> > > > > Cc: Philip Tricca <philip.b.tricca@intel.com>
> > > > > Cc: Tadeusz Struk <tadeusz.struk@intel.com>
> > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > > > 
> > > > You should use Fixes-tag e.g.
> > > > 
> > > > Fixes: <12 first chars from SHA-1> ("<short summary>")
> > > 
> > > Is this the only reason why it has not been picked up? A fixes line
> > > which triggers stable backports for something that does need to be
> > > backported?
> > 
> > Fully agree with you. Frankly, I don't really remember anymore why I
> > responded that way. My guess is that I responded that to a worng patch.
> > 
> > Please just ping immediatelly. Sometimes when dealing with dozens of
> > patches this kind of human error might happen.
> > 
> > In any case, the patch is applied.
> 
> OK, so. Gave a relook at this:
> 
> This gives checkpatch.pl error:
> 
> 0012-tpm-remove-tpm_dev_wq_lock.patch
> -------------------------------------
> ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 9e1b74a63f77 ("tpm: add support for nonblocking operation")'
> #8: 
>   9e1b74a63f776 ("tpm: add support for nonblocking operation")
> 
> total: 1 errors, 0 warnings, 7 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
> 
> Please send me a new patch with a legit fixes line. It is a fix to
> regression even if it is a cosmetic one.
> 
> I'll drop the current patch from my tree and apply a new one once
> I get it from you.
> 
> Thanks.
> 
> /Jarkko

AFAIK cc stable triggers stable backport, not fixes line alone (not
100% sure about this though). Anyway even my original response was
meant to this patch I recall now that I bumped into that checkpatch
error.

/Jarkko
Sebastian Andrzej Siewior Nov. 4, 2019, 2:39 p.m. UTC | #7
sorry for that late reply, was traveling…

On 2019-10-28 22:26:37 [+0200], Jarkko Sakkinen wrote:
> > OK, so. Gave a relook at this:
> > 
> > This gives checkpatch.pl error:
> > 
> > 0012-tpm-remove-tpm_dev_wq_lock.patch
> > -------------------------------------
> > ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 9e1b74a63f77 ("tpm: add support for nonblocking operation")'
> > #8: 
> >   9e1b74a63f776 ("tpm: add support for nonblocking operation")
> > 
> > total: 1 errors, 0 warnings, 7 lines checked
> > 
> > NOTE: For some of the reported defects, checkpatch may be able to
> >       mechanically convert to the typical style using --fix or --fix-inplace.
> > 
> > Please send me a new patch with a legit fixes line. It is a fix to
> > regression even if it is a cosmetic one.
> > 
> > I'll drop the current patch from my tree and apply a new one once
> > I get it from you.

Can you please explain what is wrong with that one? It is exactly as
suggested by the error line.

> > Thanks.
> > 
> > /Jarkko
> 
> AFAIK cc stable triggers stable backport, not fixes line alone (not
> 100% sure about this though). Anyway even my original response was
> meant to this patch I recall now that I bumped into that checkpatch
> error.

The cc: for stable and fixes are high indicators for it to be
considered. These days even a few keywords in the commit message might
let Sasha's script decide to pick/suggest a patch for stable.

> /Jarkko

Sebastian
Jerry Snitselaar Nov. 4, 2019, 5:37 p.m. UTC | #8
On Mon Nov 04 19, Sebastian Andrzej Siewior wrote:
>sorry for that late reply, was traveling…
>
>On 2019-10-28 22:26:37 [+0200], Jarkko Sakkinen wrote:
>> > OK, so. Gave a relook at this:
>> >
>> > This gives checkpatch.pl error:
>> >
>> > 0012-tpm-remove-tpm_dev_wq_lock.patch
>> > -------------------------------------
>> > ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 9e1b74a63f77 ("tpm: add support for nonblocking operation")'
>> > #8:
>> >   9e1b74a63f776 ("tpm: add support for nonblocking operation")
>> >
>> > total: 1 errors, 0 warnings, 7 lines checked
>> >
>> > NOTE: For some of the reported defects, checkpatch may be able to
>> >       mechanically convert to the typical style using --fix or --fix-inplace.
>> >
>> > Please send me a new patch with a legit fixes line. It is a fix to
>> > regression even if it is a cosmetic one.
>> >
>> > I'll drop the current patch from my tree and apply a new one once
>> > I get it from you.
>
>Can you please explain what is wrong with that one? It is exactly as
>suggested by the error line.
>
>> > Thanks.
>> >
>> > /Jarkko
>>
>> AFAIK cc stable triggers stable backport, not fixes line alone (not
>> 100% sure about this though). Anyway even my original response was
>> meant to this patch I recall now that I bumped into that checkpatch
>> error.
>
>The cc: for stable and fixes are high indicators for it to be
>considered. These days even a few keywords in the commit message might
>let Sasha's script decide to pick/suggest a patch for stable.
>
>> /Jarkko
>
>Sebastian

It looks like checkpatch is expecting the word commit to precede the hash on the same line.
I get no errors with the following:

Added in

   commit 9e1b74a63f776 ("tpm: add support for nonblocking operation")

but never actually used it.

Fixes: 9e1b74a63f776 ("tpm: add support for nonblocking operation")
Cc: Philip Tricca <philip.b.tricca@intel.com>
Cc: Tadeusz Struk <tadeusz.struk@intel.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Sebastian Andrzej Siewior Nov. 4, 2019, 5:44 p.m. UTC | #9
On 2019-11-04 10:37:09 [-0700], Jerry Snitselaar wrote:
> It looks like checkpatch is expecting the word commit to precede the hash on the same line.
> I get no errors with the following:

That would explain it. That is however not what the TIP tree and other
people do not to mention that reading wise it makes sense to keep the
word `commit' as part of the sentence and add the hash in the next line.

> Added in
> 
>   commit 9e1b74a63f776 ("tpm: add support for nonblocking operation")
> 
> but never actually used it.
> 
> Fixes: 9e1b74a63f776 ("tpm: add support for nonblocking operation")
> Cc: Philip Tricca <philip.b.tricca@intel.com>
> Cc: Tadeusz Struk <tadeusz.struk@intel.com>
> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Sebastian
Jerry Snitselaar Nov. 4, 2019, 6:27 p.m. UTC | #10
On Mon Nov 04 19, Sebastian Andrzej Siewior wrote:
>On 2019-11-04 10:37:09 [-0700], Jerry Snitselaar wrote:
>> It looks like checkpatch is expecting the word commit to precede the hash on the same line.
>> I get no errors with the following:
>
>That would explain it. That is however not what the TIP tree and other
>people do not to mention that reading wise it makes sense to keep the
>word `commit' as part of the sentence and add the hash in the next line.
>

Yes it reads better. What about the following?

Added in commit 9e1b74a63f776 ("tpm: add support for nonblocking
operation"), but never actually used it.

And then add the Fixes: line above the Cc: and Signed-off-by: ?

>> Added in
>>
>>   commit 9e1b74a63f776 ("tpm: add support for nonblocking operation")
>>
>> but never actually used it.
>>
>> Fixes: 9e1b74a63f776 ("tpm: add support for nonblocking operation")
>> Cc: Philip Tricca <philip.b.tricca@intel.com>
>> Cc: Tadeusz Struk <tadeusz.struk@intel.com>
>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
>Sebastian
Sebastian Andrzej Siewior Nov. 7, 2019, 4:10 p.m. UTC | #11
On 2019-11-04 11:27:32 [-0700], Jerry Snitselaar wrote:
> On Mon Nov 04 19, Sebastian Andrzej Siewior wrote:
> > On 2019-11-04 10:37:09 [-0700], Jerry Snitselaar wrote:
> > > It looks like checkpatch is expecting the word commit to precede the hash on the same line.
> > > I get no errors with the following:
> > 
> > That would explain it. That is however not what the TIP tree and other
> > people do not to mention that reading wise it makes sense to keep the
> > word `commit' as part of the sentence and add the hash in the next line.
> > 
> 
> Yes it reads better. What about the following?
> 
> Added in commit 9e1b74a63f776 ("tpm: add support for nonblocking
> operation"), but never actually used it.
> 
> And then add the Fixes: line above the Cc: and Signed-off-by: ?

Can please get over with? It is a simple patch. It has simple
description.

Sebastian
Jarkko Sakkinen Nov. 7, 2019, 6:35 p.m. UTC | #12
On Thu, Nov 07, 2019 at 05:10:41PM +0100, Sebastian Andrzej Siewior wrote:
> On 2019-11-04 11:27:32 [-0700], Jerry Snitselaar wrote:
> > On Mon Nov 04 19, Sebastian Andrzej Siewior wrote:
> > > On 2019-11-04 10:37:09 [-0700], Jerry Snitselaar wrote:
> > > > It looks like checkpatch is expecting the word commit to precede the hash on the same line.
> > > > I get no errors with the following:
> > > 
> > > That would explain it. That is however not what the TIP tree and other
> > > people do not to mention that reading wise it makes sense to keep the
> > > word `commit' as part of the sentence and add the hash in the next line.
> > > 
> > 
> > Yes it reads better. What about the following?
> > 
> > Added in commit 9e1b74a63f776 ("tpm: add support for nonblocking
> > operation"), but never actually used it.
> > 
> > And then add the Fixes: line above the Cc: and Signed-off-by: ?
> 
> Can please get over with? It is a simple patch. It has simple
> description.

https://lore.kernel.org/linux-integrity/20191028202419.GA7214@linux.intel.com/

I'm also cool with cc stable as long as the commit is message has the
correct format.

/Jarkko
Sebastian Andrzej Siewior Nov. 14, 2019, 11:16 a.m. UTC | #13
On 2019-11-07 20:35:03 [+0200], Jarkko Sakkinen wrote:
> On Thu, Nov 07, 2019 at 05:10:41PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2019-11-04 11:27:32 [-0700], Jerry Snitselaar wrote:
> > > On Mon Nov 04 19, Sebastian Andrzej Siewior wrote:
> > > > On 2019-11-04 10:37:09 [-0700], Jerry Snitselaar wrote:
> > > > > It looks like checkpatch is expecting the word commit to precede the hash on the same line.
> > > > > I get no errors with the following:
> > > > 
> > > > That would explain it. That is however not what the TIP tree and other
> > > > people do not to mention that reading wise it makes sense to keep the
> > > > word `commit' as part of the sentence and add the hash in the next line.
> > > > 
> > > 
> > > Yes it reads better. What about the following?
> > > 
> > > Added in commit 9e1b74a63f776 ("tpm: add support for nonblocking
> > > operation"), but never actually used it.
> > > 
> > > And then add the Fixes: line above the Cc: and Signed-off-by: ?
> > 
> > Can please get over with? It is a simple patch. It has simple
> > description.
> 
> https://lore.kernel.org/linux-integrity/20191028202419.GA7214@linux.intel.com/
> 
> I'm also cool with cc stable as long as the commit is message has the
> correct format.

This is _really_ getting ridiculous. Holding back a simple patch just
because checkpatch says that the word `commit' is not in a new line. It
is more readable that way not to mention line with the commit id is
getting really long. This is a stupid checkpatch rule which is enforced
here.

> /Jarkko

Sebastian
Jarkko Sakkinen Nov. 15, 2019, 5:34 p.m. UTC | #14
On Thu, Nov 14, 2019 at 12:16:12PM +0100, Sebastian Andrzej Siewior wrote:
> On 2019-11-07 20:35:03 [+0200], Jarkko Sakkinen wrote:
> > On Thu, Nov 07, 2019 at 05:10:41PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 2019-11-04 11:27:32 [-0700], Jerry Snitselaar wrote:
> > > > On Mon Nov 04 19, Sebastian Andrzej Siewior wrote:
> > > > > On 2019-11-04 10:37:09 [-0700], Jerry Snitselaar wrote:
> > > > > > It looks like checkpatch is expecting the word commit to precede the hash on the same line.
> > > > > > I get no errors with the following:
> > > > > 
> > > > > That would explain it. That is however not what the TIP tree and other
> > > > > people do not to mention that reading wise it makes sense to keep the
> > > > > word `commit' as part of the sentence and add the hash in the next line.
> > > > > 
> > > > 
> > > > Yes it reads better. What about the following?
> > > > 
> > > > Added in commit 9e1b74a63f776 ("tpm: add support for nonblocking
> > > > operation"), but never actually used it.
> > > > 
> > > > And then add the Fixes: line above the Cc: and Signed-off-by: ?
> > > 
> > > Can please get over with? It is a simple patch. It has simple
> > > description.
> > 
> > https://lore.kernel.org/linux-integrity/20191028202419.GA7214@linux.intel.com/
> > 
> > I'm also cool with cc stable as long as the commit is message has the
> > correct format.
> 
> This is _really_ getting ridiculous. Holding back a simple patch just
> because checkpatch says that the word `commit' is not in a new line. It
> is more readable that way not to mention line with the commit id is
> getting really long. This is a stupid checkpatch rule which is enforced
> here.

I'm not sure why formatting a commit message properly is ridicilous.

If it is a bug fix, then it should have fixes tag.

/Jarkko
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 5eecad233ea1d..eca96e8c669c9 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -25,7 +25,6 @@ 
 #include "tpm-dev.h"
 
 static struct workqueue_struct *tpm_dev_wq;
-static DEFINE_MUTEX(tpm_dev_wq_lock);
 
 static void tpm_async_work(struct work_struct *work)
 {