Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] tpm: Switch to platform_get_irq_optional()
@ 2019-10-19  9:45 Hans de Goede
  2019-10-21 14:05 ` Jason Gunthorpe
  2019-10-21 15:49 ` Jarkko Sakkinen
  0 siblings, 2 replies; 14+ messages in thread
From: Hans de Goede @ 2019-10-19  9:45 UTC (permalink / raw)
  To: Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe
  Cc: Hans de Goede, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
as the IRQ usage in the tpm_tis driver is optional, this is undesirable.

Specifically this leads to this new false-positive error being logged:
[    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found

This commit switches to platform_get_irq_optional(), which does not log
an error, fixing this.

Cc: <stable@vger.kernel.org> # 5.4.x
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/char/tpm/tpm_tis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index e4fdde93ed4c..e7df342a317d 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -286,7 +286,7 @@ static int tpm_tis_plat_probe(struct platform_device *pdev)
 	}
 	tpm_info.res = *res;
 
-	tpm_info.irq = platform_get_irq(pdev, 0);
+	tpm_info.irq = platform_get_irq_optional(pdev, 0);
 	if (tpm_info.irq <= 0) {
 		if (pdev != force_pdev)
 			tpm_info.irq = -1;
-- 
2.23.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-19  9:45 [PATCH] tpm: Switch to platform_get_irq_optional() Hans de Goede
@ 2019-10-21 14:05 ` Jason Gunthorpe
  2019-10-23 11:32   ` Jarkko Sakkinen
  2019-10-21 15:49 ` Jarkko Sakkinen
  1 sibling, 1 reply; 14+ messages in thread
From: Jason Gunthorpe @ 2019-10-21 14:05 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Peter Huewe, Jarkko Sakkinen, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
> platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
> as the IRQ usage in the tpm_tis driver is optional, this is undesirable.

This should have a fixes line for the above, or maybe the commit that
addtion the _optional version..

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-19  9:45 [PATCH] tpm: Switch to platform_get_irq_optional() Hans de Goede
  2019-10-21 14:05 ` Jason Gunthorpe
@ 2019-10-21 15:49 ` Jarkko Sakkinen
  2019-10-21 15:56   ` Hans de Goede
  1 sibling, 1 reply; 14+ messages in thread
From: Jarkko Sakkinen @ 2019-10-21 15:49 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
> platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
> as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
> 
> Specifically this leads to this new false-positive error being logged:
> [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
> 
> This commit switches to platform_get_irq_optional(), which does not log
> an error, fixing this.
> 
> Cc: <stable@vger.kernel.org> # 5.4.x

Incorrect format (should be wo '<' and '>').

Also, not sure why this should be backported to stable kernel anyway.

/Jarkko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-21 15:49 ` Jarkko Sakkinen
@ 2019-10-21 15:56   ` Hans de Goede
  2019-10-23 11:37     ` Jarkko Sakkinen
  0 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2019-10-21 15:56 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

Hi,

On 21-10-2019 17:49, Jarkko Sakkinen wrote:
> On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
>> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
>> platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
>> as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
>>
>> Specifically this leads to this new false-positive error being logged:
>> [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
>>
>> This commit switches to platform_get_irq_optional(), which does not log
>> an error, fixing this.
>>
>> Cc: <stable@vger.kernel.org> # 5.4.x
> 
> Incorrect format (should be wo '<' and '>').

According to:

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

the '<' and '>' should be added when adding a # <kerner>

> Also, not sure why this should be backported to stable kernel anyway.

Because false-positive error messages are bad and cause users to
file false-positive bug-reports.

Regards,

Hans


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-21 14:05 ` Jason Gunthorpe
@ 2019-10-23 11:32   ` Jarkko Sakkinen
  2019-10-24 15:36     ` Jason Gunthorpe
  0 siblings, 1 reply; 14+ messages in thread
From: Jarkko Sakkinen @ 2019-10-23 11:32 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Hans de Goede, Peter Huewe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Mon, Oct 21, 2019 at 11:05:02AM -0300, Jason Gunthorpe wrote:
> On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
> > Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
> > platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
> > as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
> 
> This should have a fixes line for the above, or maybe the commit that
> addtion the _optional version..

Is this fixing something?

/Jarkko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-21 15:56   ` Hans de Goede
@ 2019-10-23 11:37     ` Jarkko Sakkinen
  2019-10-23 14:32       ` Hans de Goede
  0 siblings, 1 reply; 14+ messages in thread
From: Jarkko Sakkinen @ 2019-10-23 11:37 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Mon, Oct 21, 2019 at 05:56:56PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 21-10-2019 17:49, Jarkko Sakkinen wrote:
> > On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
> > > Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
> > > platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
> > > as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
> > > 
> > > Specifically this leads to this new false-positive error being logged:
> > > [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
> > > 
> > > This commit switches to platform_get_irq_optional(), which does not log
> > > an error, fixing this.
> > > 
> > > Cc: <stable@vger.kernel.org> # 5.4.x
> > 
> > Incorrect format (should be wo '<' and '>').
> 
> According to:
> 
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> 
> the '<' and '>' should be added when adding a # <kerner>

OK, right so it was. This first patch that I'm reviewing with such
commit.

> > Also, not sure why this should be backported to stable kernel anyway.
> 
> Because false-positive error messages are bad and cause users to
> file false-positive bug-reports.

Neither categorizes into a regression albeit being unfortunate
glitches.

/Jarkko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-23 11:37     ` Jarkko Sakkinen
@ 2019-10-23 14:32       ` Hans de Goede
  2019-10-24 14:25         ` Jarkko Sakkinen
  0 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2019-10-23 14:32 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

Hi,

On 23-10-2019 13:37, Jarkko Sakkinen wrote:
> On Mon, Oct 21, 2019 at 05:56:56PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 21-10-2019 17:49, Jarkko Sakkinen wrote:
>>> On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
>>>> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
>>>> platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
>>>> as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
>>>>
>>>> Specifically this leads to this new false-positive error being logged:
>>>> [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
>>>>
>>>> This commit switches to platform_get_irq_optional(), which does not log
>>>> an error, fixing this.
>>>>
>>>> Cc: <stable@vger.kernel.org> # 5.4.x
>>>
>>> Incorrect format (should be wo '<' and '>').
>>
>> According to:
>>
>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>
>> the '<' and '>' should be added when adding a # <kerner>
> 
> OK, right so it was. This first patch that I'm reviewing with such
> commit.
> 
>>> Also, not sure why this should be backported to stable kernel anyway.
>>
>> Because false-positive error messages are bad and cause users to
>> file false-positive bug-reports.
> 
> Neither categorizes into a regression albeit being unfortunate
> glitches.

The stable series also does other small fixes, such as adding new
pci/usb-ids, etc. This clearly falls within this. IMHO ideally this
should go to a 5.4-rc# making the whole discussion moot, but since
I was afraid it would not make 5.4, I added the Cc: stable.

Regards,

Hans


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-23 14:32       ` Hans de Goede
@ 2019-10-24 14:25         ` Jarkko Sakkinen
  2019-10-24 14:27           ` Hans de Goede
  0 siblings, 1 reply; 14+ messages in thread
From: Jarkko Sakkinen @ 2019-10-24 14:25 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Wed, Oct 23, 2019 at 04:32:57PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 23-10-2019 13:37, Jarkko Sakkinen wrote:
> > On Mon, Oct 21, 2019 at 05:56:56PM +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 21-10-2019 17:49, Jarkko Sakkinen wrote:
> > > > On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
> > > > > Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
> > > > > platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
> > > > > as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
> > > > > 
> > > > > Specifically this leads to this new false-positive error being logged:
> > > > > [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
> > > > > 
> > > > > This commit switches to platform_get_irq_optional(), which does not log
> > > > > an error, fixing this.
> > > > > 
> > > > > Cc: <stable@vger.kernel.org> # 5.4.x
> > > > 
> > > > Incorrect format (should be wo '<' and '>').
> > > 
> > > According to:
> > > 
> > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > > 
> > > the '<' and '>' should be added when adding a # <kerner>
> > 
> > OK, right so it was. This first patch that I'm reviewing with such
> > commit.
> > 
> > > > Also, not sure why this should be backported to stable kernel anyway.
> > > 
> > > Because false-positive error messages are bad and cause users to
> > > file false-positive bug-reports.
> > 
> > Neither categorizes into a regression albeit being unfortunate
> > glitches.
> 
> The stable series also does other small fixes, such as adding new
> pci/usb-ids, etc. This clearly falls within this. IMHO ideally this
> should go to a 5.4-rc# making the whole discussion moot, but since
> I was afraid it would not make 5.4, I added the Cc: stable.

I get adding PCI/USB id as it extends the hardware support for the
stable kernel without risking its stability. This patch is factors
less useful.

/Jarkko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-24 14:25         ` Jarkko Sakkinen
@ 2019-10-24 14:27           ` Hans de Goede
  2019-10-24 19:09             ` Jarkko Sakkinen
  0 siblings, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2019-10-24 14:27 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

Hi,

On 24-10-2019 16:25, Jarkko Sakkinen wrote:
> On Wed, Oct 23, 2019 at 04:32:57PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 23-10-2019 13:37, Jarkko Sakkinen wrote:
>>> On Mon, Oct 21, 2019 at 05:56:56PM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 21-10-2019 17:49, Jarkko Sakkinen wrote:
>>>>> On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
>>>>>> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
>>>>>> platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
>>>>>> as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
>>>>>>
>>>>>> Specifically this leads to this new false-positive error being logged:
>>>>>> [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
>>>>>>
>>>>>> This commit switches to platform_get_irq_optional(), which does not log
>>>>>> an error, fixing this.
>>>>>>
>>>>>> Cc: <stable@vger.kernel.org> # 5.4.x
>>>>>
>>>>> Incorrect format (should be wo '<' and '>').
>>>>
>>>> According to:
>>>>
>>>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>>>
>>>> the '<' and '>' should be added when adding a # <kerner>
>>>
>>> OK, right so it was. This first patch that I'm reviewing with such
>>> commit.
>>>
>>>>> Also, not sure why this should be backported to stable kernel anyway.
>>>>
>>>> Because false-positive error messages are bad and cause users to
>>>> file false-positive bug-reports.
>>>
>>> Neither categorizes into a regression albeit being unfortunate
>>> glitches.
>>
>> The stable series also does other small fixes, such as adding new
>> pci/usb-ids, etc. This clearly falls within this. IMHO ideally this
>> should go to a 5.4-rc# making the whole discussion moot, but since
>> I was afraid it would not make 5.4, I added the Cc: stable.
> 
> I get adding PCI/USB id as it extends the hardware support for the
> stable kernel without risking its stability. This patch is factors
> less useful.

It also has about 0% chance of causing regressions and it does
help to avoid false--positive bug reports.

TBH I'm quite surprised we are even having this discussion...

Regards,

Hans


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-23 11:32   ` Jarkko Sakkinen
@ 2019-10-24 15:36     ` Jason Gunthorpe
  0 siblings, 0 replies; 14+ messages in thread
From: Jason Gunthorpe @ 2019-10-24 15:36 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Hans de Goede, Peter Huewe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Wed, Oct 23, 2019 at 02:32:48PM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 21, 2019 at 11:05:02AM -0300, Jason Gunthorpe wrote:
> > On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
> > > Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
> > > platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
> > > as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
> > 
> > This should have a fixes line for the above, or maybe the commit that
> > addtion the _optional version..
> 
> Is this fixing something?

Yes, an earlier commit caused new bogus warnings to appear, this is
fixing that regression

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-24 14:27           ` Hans de Goede
@ 2019-10-24 19:09             ` Jarkko Sakkinen
  2019-10-24 19:11               ` Jason Gunthorpe
  2019-10-25  9:13               ` Hans de Goede
  0 siblings, 2 replies; 14+ messages in thread
From: Jarkko Sakkinen @ 2019-10-24 19:09 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Thu, Oct 24, 2019 at 04:27:24PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 24-10-2019 16:25, Jarkko Sakkinen wrote:
> > On Wed, Oct 23, 2019 at 04:32:57PM +0200, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 23-10-2019 13:37, Jarkko Sakkinen wrote:
> > > > On Mon, Oct 21, 2019 at 05:56:56PM +0200, Hans de Goede wrote:
> > > > > Hi,
> > > > > 
> > > > > On 21-10-2019 17:49, Jarkko Sakkinen wrote:
> > > > > > On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
> > > > > > > Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
> > > > > > > platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
> > > > > > > as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
> > > > > > > 
> > > > > > > Specifically this leads to this new false-positive error being logged:
> > > > > > > [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
> > > > > > > 
> > > > > > > This commit switches to platform_get_irq_optional(), which does not log
> > > > > > > an error, fixing this.
> > > > > > > 
> > > > > > > Cc: <stable@vger.kernel.org> # 5.4.x
> > > > > > 
> > > > > > Incorrect format (should be wo '<' and '>').
> > > > > 
> > > > > According to:
> > > > > 
> > > > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > > > > 
> > > > > the '<' and '>' should be added when adding a # <kerner>
> > > > 
> > > > OK, right so it was. This first patch that I'm reviewing with such
> > > > commit.
> > > > 
> > > > > > Also, not sure why this should be backported to stable kernel anyway.
> > > > > 
> > > > > Because false-positive error messages are bad and cause users to
> > > > > file false-positive bug-reports.
> > > > 
> > > > Neither categorizes into a regression albeit being unfortunate
> > > > glitches.
> > > 
> > > The stable series also does other small fixes, such as adding new
> > > pci/usb-ids, etc. This clearly falls within this. IMHO ideally this
> > > should go to a 5.4-rc# making the whole discussion moot, but since
> > > I was afraid it would not make 5.4, I added the Cc: stable.
> > 
> > I get adding PCI/USB id as it extends the hardware support for the
> > stable kernel without risking its stability. This patch is factors
> > less useful.
> 
> It also has about 0% chance of causing regressions and it does
> help to avoid false--positive bug reports.
> 
> TBH I'm quite surprised we are even having this discussion...
> 
> Regards,
> 
> Hans

Why do you think that way?

I mean the commit does not even have a fixes line. It already obviously
implies that this kind of discussion is mandatory. Your reasoning in
this discussion does make sense. The problem is really the commit
message supplied.

I'd guess something like this would be more appropriate:

"
platform_get_irq() calls dev_err() on an error. As the IRQ usage in the
tpm_tis driver is optional, this is undesirable.

Specifically this leads to this new false-positive error being logged:
[    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found

This commit switches to platform_get_irq_optional(), which does not log
an error, fixing this.

Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()"
Cc: <stable@vger.kernel.org> # 5.4.x
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
"

You should confirm this by sending v2 either what I proposed unmodified
or with the changes that you see appropriate.

/Jarkko

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-24 19:09             ` Jarkko Sakkinen
@ 2019-10-24 19:11               ` Jason Gunthorpe
  2019-10-25  9:13               ` Hans de Goede
  1 sibling, 0 replies; 14+ messages in thread
From: Jason Gunthorpe @ 2019-10-24 19:11 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Hans de Goede, Peter Huewe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Thu, Oct 24, 2019 at 10:09:42PM +0300, Jarkko Sakkinen wrote:
> Why do you think that way?
> 
> I mean the commit does not even have a fixes line. It already obviously
> implies that this kind of discussion is mandatory. Your reasoning in
> this discussion does make sense. The problem is really the commit
> message supplied.
> 
> I'd guess something like this would be more appropriate:
> 
> "
> platform_get_irq() calls dev_err() on an error. As the IRQ usage in the
> tpm_tis driver is optional, this is undesirable.
> 
> Specifically this leads to this new false-positive error being logged:
> [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
> 
> This commit switches to platform_get_irq_optional(), which does not log
> an error, fixing this.
> 
> Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()"
> Cc: <stable@vger.kernel.org> # 5.4.x
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> "

This is much better, yes

Jason

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-24 19:09             ` Jarkko Sakkinen
  2019-10-24 19:11               ` Jason Gunthorpe
@ 2019-10-25  9:13               ` Hans de Goede
  2019-10-28 20:47                 ` Jarkko Sakkinen
  1 sibling, 1 reply; 14+ messages in thread
From: Hans de Goede @ 2019-10-25  9:13 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

Hi,

On 24-10-2019 21:09, Jarkko Sakkinen wrote:
> On Thu, Oct 24, 2019 at 04:27:24PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 24-10-2019 16:25, Jarkko Sakkinen wrote:
>>> On Wed, Oct 23, 2019 at 04:32:57PM +0200, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 23-10-2019 13:37, Jarkko Sakkinen wrote:
>>>>> On Mon, Oct 21, 2019 at 05:56:56PM +0200, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 21-10-2019 17:49, Jarkko Sakkinen wrote:
>>>>>>> On Sat, Oct 19, 2019 at 11:45:28AM +0200, Hans de Goede wrote:
>>>>>>>> Since commit 7723f4c5ecdb ("driver core: platform: Add an error message to
>>>>>>>> platform_get_irq*()"), platform_get_irq() will call dev_err() on an error,
>>>>>>>> as the IRQ usage in the tpm_tis driver is optional, this is undesirable.
>>>>>>>>
>>>>>>>> Specifically this leads to this new false-positive error being logged:
>>>>>>>> [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
>>>>>>>>
>>>>>>>> This commit switches to platform_get_irq_optional(), which does not log
>>>>>>>> an error, fixing this.
>>>>>>>>
>>>>>>>> Cc: <stable@vger.kernel.org> # 5.4.x
>>>>>>>
>>>>>>> Incorrect format (should be wo '<' and '>').
>>>>>>
>>>>>> According to:
>>>>>>
>>>>>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>>>>>
>>>>>> the '<' and '>' should be added when adding a # <kerner>
>>>>>
>>>>> OK, right so it was. This first patch that I'm reviewing with such
>>>>> commit.
>>>>>
>>>>>>> Also, not sure why this should be backported to stable kernel anyway.
>>>>>>
>>>>>> Because false-positive error messages are bad and cause users to
>>>>>> file false-positive bug-reports.
>>>>>
>>>>> Neither categorizes into a regression albeit being unfortunate
>>>>> glitches.
>>>>
>>>> The stable series also does other small fixes, such as adding new
>>>> pci/usb-ids, etc. This clearly falls within this. IMHO ideally this
>>>> should go to a 5.4-rc# making the whole discussion moot, but since
>>>> I was afraid it would not make 5.4, I added the Cc: stable.
>>>
>>> I get adding PCI/USB id as it extends the hardware support for the
>>> stable kernel without risking its stability. This patch is factors
>>> less useful.
>>
>> It also has about 0% chance of causing regressions and it does
>> help to avoid false--positive bug reports.
>>
>> TBH I'm quite surprised we are even having this discussion...
>>
>> Regards,
>>
>> Hans
> 
> Why do you think that way?
> 
> I mean the commit does not even have a fixes line. It already obviously
> implies that this kind of discussion is mandatory. Your reasoning in
> this discussion does make sense. The problem is really the commit
> message supplied.
> 
> I'd guess something like this would be more appropriate:
> 
> "
> platform_get_irq() calls dev_err() on an error. As the IRQ usage in the
> tpm_tis driver is optional, this is undesirable.
> 
> Specifically this leads to this new false-positive error being logged:
> [    5.135413] tpm_tis MSFT0101:00: IRQ index 0 not found
> 
> This commit switches to platform_get_irq_optional(), which does not log
> an error, fixing this.
> 
> Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()"
> Cc: <stable@vger.kernel.org> # 5.4.x
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> "
> 
> You should confirm this by sending v2 either what I proposed unmodified
> or with the changes that you see appropriate.

Ok, this is fine with me, I will send v2 with the updated commit msg right away.

Regards,

Hans


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] tpm: Switch to platform_get_irq_optional()
  2019-10-25  9:13               ` Hans de Goede
@ 2019-10-28 20:47                 ` Jarkko Sakkinen
  0 siblings, 0 replies; 14+ messages in thread
From: Jarkko Sakkinen @ 2019-10-28 20:47 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	linux-integrity, stable

On Fri, Oct 25, 2019 at 11:13:21AM +0200, Hans de Goede wrote:
> Ok, this is fine with me, I will send v2 with the updated commit msg right away.

Thank you, appreciate it.

/Jarkko

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-19  9:45 [PATCH] tpm: Switch to platform_get_irq_optional() Hans de Goede
2019-10-21 14:05 ` Jason Gunthorpe
2019-10-23 11:32   ` Jarkko Sakkinen
2019-10-24 15:36     ` Jason Gunthorpe
2019-10-21 15:49 ` Jarkko Sakkinen
2019-10-21 15:56   ` Hans de Goede
2019-10-23 11:37     ` Jarkko Sakkinen
2019-10-23 14:32       ` Hans de Goede
2019-10-24 14:25         ` Jarkko Sakkinen
2019-10-24 14:27           ` Hans de Goede
2019-10-24 19:09             ` Jarkko Sakkinen
2019-10-24 19:11               ` Jason Gunthorpe
2019-10-25  9:13               ` Hans de Goede
2019-10-28 20:47                 ` Jarkko Sakkinen

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org
	public-inbox-index linux-integrity

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git