Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7cafafd3-e7a7-96db-0f8f-ac5c8bc94d13@gmail.com>
Date:   Sun, 7 May 2023 19:13:38 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Content-Language: en-US, en-GB
To:     Jonathan Cameron <jic23@kernel.org>,
        "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
Cc:     Lars-Peter Clausen <lars@metafoo.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Shreeya Patel <shreeya.patel@collabora.com>,
        Zhigang Shi <Zhigang.Shi@liteon.com>,
        Paul Gazzillo <paul@pgazz.com>,
        "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Andi Shyti <andi.shyti@kernel.org>
References: <cover.1682495921.git.mazziesaccount@gmail.com>
 <fb35de40a3908988f5f83e25d17119e6944d289b.1682495921.git.mazziesaccount@gmail.com>
 <20230501152014.7789aa42@jic23-huawei>
 <91463df1-5aba-484a-92ea-f8979ec30535@fi.rohmeurope.com>
 <20230507152236.46aba096@jic23-huawei>
From:   Matti Vaittinen <mazziesaccount@gmail.com>
Subject: Re: [PATCH v3 4/5] iio: light: ROHM BU27008 color sensor
In-Reply-To: <20230507152236.46aba096@jic23-huawei>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/7/23 17:22, Jonathan Cameron wrote:

>>> If there is nothing to do in the actual interrupt as it's a data ready
>>> only signal, then you should just call iio_trigger_poll() in the top half and
>>> use devm_request_irq() only as there is no thread in this interrupt (though
>>> there is one for the interrupt below the software interrupt chip).
>>
>> I haven't tested this yet so please ignore me if I am writing nonsense -
>> but... The BU27008 will keep the IRQ line asserted until a register is
>> read. We can't read the register form HW-IRQ so we need to keep the IRQ
>> disabled until the threaded trigger handler is ran. With the setup we
>> have here, the IRQF_ONESHOT, took care of this. I assume that changing
>> to call the iio_poll_trigger() from top-half means I need to explicitly
>> disable the IRQ and re-enable it at the end of the trigger thread after
>> reading the register which debounces the IRQ line?
> 
> Hmm. I'm trying to remember how this works (wrote this a very long time ago).
> I'm fairly sure it's not an issue because we use IRQF_ONESHOT down one level
> so exercise the same prevention of the threads triggering multiple times etc > 
https://elixir.bootlin.com/linux/latest/source/drivers/iio/buffer/industrialio-triggered-buffer.c#L57
> 
> It doesn't matter if the device interrupt fires again as it will still be masked
> at our software irqchip level and will then get queued up and the thread will
> run again.

After reading this I am not at all sure I am using the trigger 
correctly. I see the iio_trigger_attach_poll_func() registering threaded 
handler with the IRQF_ONESHOT which is stored in the 
iio_alloc_pollfunc() as you pointed above.

The iio_trigger_attach_poll_func() is called from sysfs callback when 
trigger is enabled. So, if this is supposed to be the device IRQ, then I 
am not at all sure the driver should be requesting the IRQ at the 
probe(). If it is not the device's IRQ, then I guess the IRQF_ONESHOT 
passed in here won't help. I need to try seeing some examples how other 
drivers are using the triggers. Getting back to this tomorrow...

Yours,
	-- Matti.

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~