Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Fri, 13 May 2022 21:08:48 +0300
From:   Jarkko Sakkinen <jarkko@kernel.org>
To:     Lino Sanfilippo <LinoSanfilippo@gmx.de>
Cc:     peterhuewe@gmx.de, jgg@ziepe.ca, stefanb@linux.vnet.ibm.com,
        linux@mniewoehner.de, linux-integrity@vger.kernel.org,
        linux-kernel@vger.kernel.org, lukas@wunner.de,
        p.rosenberger@kunbus.com, Lino Sanfilippo <l.sanfilippo@kunbus.com>
Subject: Re: [PATCH v4 1/6] tpm, tpm_tis_spi: Request threaded irq
Message-ID: <Yn6esMyUl+QhECq+@kernel.org>
References: <20220509080559.4381-1-LinoSanfilippo@gmx.de>
 <20220509080559.4381-2-LinoSanfilippo@gmx.de>
 <YnucgDH3I87RI8PN@kernel.org>
 <486cec01-ec02-3f11-0b81-037e0700c503@gmx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <486cec01-ec02-3f11-0b81-037e0700c503@gmx.de>
Precedence: bulk

On Wed, May 11, 2022 at 09:18:39PM +0200, Lino Sanfilippo wrote:
> Hi,
> 
> On 11.05.22 at 13:22, Jarkko Sakkinen wrote:
> > On Mon, May 09, 2022 at 10:05:54AM +0200, Lino Sanfilippo wrote:
> >> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> >>
> >> Interrupt handling at least includes reading and writing the interrupt
> >> status register within the interrupt routine. Since accesses over the SPI
> >> bus are synchronized by a mutex, request a threaded interrupt handler to
> >> ensure a sleepable context during interrupt processing.
> >>
> >> Fixes: 1a339b658d9d ("tpm_tis_spi: Pass the SPI IRQ down to the driver")
> >> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> >
> > When you state that it needs a sleepable context, you should bring a
> > context why it needs it. This not to disregard the code change overally but
> > you cannot make even the most obvious claim without backing data.
> >
> 
> so what kind of backing data do you have in mind? Would it help to emphasize more
> that the irq handler is running in hard irq context in the current code and thus
> must not access registers over SPI since SPI uses a mutex (I consider it as basic
> knowledge that a mutex must not be taken in hard irq context)?

There's zero mention about specific lock you are talking about. Providing
the basic knowledge what you are trying to do is the whole point of the
commit message in the first place. I'd presume this patch is related to the
use bus_lock_mutex but it is fully ingored here.

BR, Jarkko