Date:   Sat, 4 Mar 2023 16:42:19 +0000
From:   Jonathan Cameron <jic23@kernel.org>
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Mehdi Djait <mehdi.djait.k@gmail.com>, lars@metafoo.de,
        linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] iio: Improve the kernel-doc of iio_trigger_poll
Message-ID: <20230304164219.2ee62070@jic23-huawei>
In-Reply-To: <ZADNexu/8SnwfI0B@smile.fi.intel.com>
References: <cover.1677761379.git.mehdi.djait.k@gmail.com>
        <bd84fc17e9d22eab998bf48720297f9a77689f45.1677761379.git.mehdi.djait.k@gmail.com>
        <ZADGnRILByq1/1ch@smile.fi.intel.com>
        <ZADMRsDgxKZ/UfJj@carbian>
        <ZADNexu/8SnwfI0B@smile.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 2 Mar 2023 18:23:23 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, Mar 02, 2023 at 05:18:14PM +0100, Mehdi Djait wrote:
> > On Thu, Mar 02, 2023 at 05:54:05PM +0200, Andy Shevchenko wrote:  
> > > On Thu, Mar 02, 2023 at 02:04:35PM +0100, Mehdi Djait wrote:  
> > > > Move the kernel-doc of the function to industrialio-trigger.c
> > > > Add a note on the context where the function is expected to be called.  
> 
> ...
> 
> > > > v2:
> > > > - Changed the expected context of from interrupt to hard IRQ context  
> > > 
> > > Thank you for an update.
> > > 
> > > But it seems I messed up with this and my previous remark shouldn't be
> > > taken into consideration.
> > > 
> > > The "relevant hardware interrupt handler" may be hard and threaded IRQ context,
> > > which looks like your first version was correct.
> > > 
> > > Let's wait for Jonathan opinion on this as he is a native speaker.  
> > 
> > If I understood the function correctly I think you were right. It should
> > be hard IRQ context
> > 
> > The relevant functions calls:
> > iio_trigger_poll --> generic_handle_irq --> handle_irq_desc
> > 
> > handle_irq_desc: returns Operation not permitted if !in_hardirq() && handle_enforce_irqctx 
> > and it is the reason why the sysfs trigger uses the irq_framework to call iio_trigger_poll 
> > from hard IRQ context [1][2]  
> 
> Cool, thank you for elaboration!
> 
> In any case it's up to Jonathan now what to do. With your explanation it seems
> correct to phrase as you did in v2. Hence,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

It is indeed hard interrupt context that matters here so I'm fine with this version.
Though I'm going to tweak it to drop the empty line at the end of the comment block
whilst applying it.

Thanks,

Jonathan

> 
> > [1] https://lwn.net/Articles/411605/ 
> > [2] https://lore.kernel.org/all/1346922337-17088-1-git-send-email-lars@metafoo.de/  
>