Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755275AbbKJSHT (ORCPT ); Tue, 10 Nov 2015 13:07:19 -0500 Received: from smtp-out-224.synserver.de ([212.40.185.224]:1055 "EHLO smtp-out-188.synserver.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754949AbbKJSHQ (ORCPT ); Tue, 10 Nov 2015 13:07:16 -0500 X-SynServer-TrustedSrc: 1 X-SynServer-AuthUser: lars@metafoo.de X-SynServer-PPID: 8056 Message-ID: <56423245.1040602@metafoo.de> Date: Tue, 10 Nov 2015 19:07:01 +0100 From: Lars-Peter Clausen User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.8.0 MIME-Version: 1.0 To: Grygorii Strashko , Jon Hunter , Thomas Gleixner CC: Jason Cooper , Marc Zyngier , Stephen Warren , Thierry Reding , Kevin Hilman , Geert Uytterhoeven , LKML , linux-tegra@vger.kernel.org, Soren Brinkmann , Linus Walleij , Alexandre Courbot Subject: Re: [RFC PATCH 1/2] genirq: Add runtime resume/suspend support for IRQ chips References: <1447166377-19707-1-git-send-email-jonathanh@nvidia.com> <1447166377-19707-2-git-send-email-jonathanh@nvidia.com> <56421421.8070807@nvidia.com> <56421FA5.4020801@ti.com> In-Reply-To: <56421FA5.4020801@ti.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2364 Lines: 58 On 11/10/2015 05:47 PM, Grygorii Strashko wrote: [...] >> I was trying to simplify matters by placing the resume call in >> __setup_irq() as opposed to requested_threaded_irq(). However, the would >> mean the resume is inside the bus_lock and may be I should not assume >> that I can sleep here. >> >>> Can you folks please agree on something which is correct and complete? >> >> Soren I am happy to defer to your patch and drop this. My only comment >> would be what about the request_percpu_irq() path in your patch? >> > > I have the same comment here as I asked Soren: > 1) There are no restrictions to call irq set_irq_type() whenever, > as result HW can be accessed before request_x_irq()/__setup_irq(). > And this is used quite widely now :( > Changing the configuration of a resource that is not owned seems to be fairly broken. In the worst case this will overwrite the configuration that was set by owner of the resource. Especially those that call irq_set_irq_type() directly before request_irq(), given that you supply the trigger type to request_irq() which will make sure that there are no conflicts and the configure. This is a bit like calling gpio_set_direction() before you call gpio_request(), which will also have PM issues. > For example, during OF boot: > > [a] irq_create_of_mapping() > - irq_create_fwspec_mapping() > - irq_set_irq_type() > > or > irq_set_irq_type(irq, IRQ_TYPE_LEVEL_HIGH); > irq_set_chained_handler(irq, mx31ads_expio_irq_handler); > > or > irq_set_irq_type(alarm_irq, IRQ_TYPE_EDGE_BOTH); > err = devm_request_irq(&pdev->dev, alarm_irq, fan_alarm_irq_handler, > (there are ~200 occurrences of irq set_irq_type in Kernel) > > 2) if i'm not wrong, the same is valid for irq_set_irq_wake() and irq_set_affinity() > > I'm not saying all these code is correct, but that what's now in kernel :( > I've tried to test Soren's patch with omap-gpio and immediately hit case [a] :.( All functions for which are part of the public API and for which it is legal to call them without calling request_irq() (or similar) first will need to have pm_get()/pm_put(). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/