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:   Mon, 06 Jun 2022 09:49:43 +0100
Message-ID: <8735gi5g7s.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc:     tglx@linutronix.de, bjorn.andersson@linaro.org,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH 3/3] genirq: Check for trigger type mismatch in __setup_irq()
In-Reply-To: <20220530080842.37024-4-manivannan.sadhasivam@linaro.org>
References: <20220530080842.37024-1-manivannan.sadhasivam@linaro.org>
        <20220530080842.37024-4-manivannan.sadhasivam@linaro.org>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Mon, 30 May 2022 09:08:42 +0100,
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote:
> 
> Currently, if the trigger type defined by the platform like DT does not
> match the driver requested trigger type, the below warning is shown
> during platform_get_irq() but only during the second time of the drive
> probe (due to probe deferral or module unload/load).
> 
> irq: type mismatch, failed to map hwirq-9 for interrupt-controller@b220000!
> 
> Consider a typical usecase of requesting an IRQ in a driver:
> 
> ```
> 	/* Assume DT has set the trigger type to IRQF_TYPE_LEVEL_HIGH */
> 
> 	q6v5->wdog_irq = platform_get_irq_byname(pdev, "wdog");
> 	if (q6v5->wdog_irq <= 0)
> 		return q6v5->wdog_irq;
> 
> 	ret = devm_request_threaded_irq(&pdev->dev, q6v5->wdog_irq,
> 					NULL, q6v5_wdog_interrupt,
> 					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> 					"q6v5 wdog", q6v5);
> 	if (ret) {
> 		dev_err(&pdev->dev, "failed to acquire wdog IRQ\n");
> 		return ret;
> 	}
> ```
> 
> For the first time probe of a driver, platform_get_irq_byname() does not
> return an error and it sets the platform requested trigger type. Then,
> request_irq() also does not check for the trigger type mismatch and sets
> the driver requested trigger type. Later if the driver gets probed again,
> platform_get_irq() throws the "type mismatch" warning and fails.
> 
> Ideally, request_irq() should throw the error during the first time itself,
> when it detects the trigger type mismatch. So let's add a check in
> __setup_irq() for checking the trigger type mismatch.

No, that's wrong. The whole point is to be able to *override* the
default that is exposed by the device tree or ACPI. We have countless
examples of that, and they cannot be broken.

If the issue exists after an unload, then it is a unload time that the
previous behaviour should be restored.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.