* [PATCH v2] serial: 8250_of: Fix for lack of interrupt support
@ 2018-08-30 9:08 John Garry
2018-08-30 14:19 ` Rob Herring
2018-08-30 18:21 ` Alexander Sverdlin
0 siblings, 2 replies; 4+ messages in thread
From: John Garry @ 2018-08-30 9:08 UTC (permalink / raw)
To: gregkh
Cc: alexander.sverdlin, jslaby, robh, joel, kurt, yamada.masahiro,
linux-serial, linux-kernel, linuxarm, John Garry
In commit c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ"), a
check was added for the UART driver being probed prior to the parent IRQ
controller.
Unfortunately this breaks certain boards which have no interrupt support,
like Huawei D03.
Indeed, the 8250 DT bindings state that interrupts should be supported -
not must.
To fix, switch from irq_of_parse_and_map() to of_irq_get(), which
does relay whether the IRQ host controller domain is not ready, i.e.
defer probe, instead of assuming it.
Fixes: c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ")
Signed-off-by: John Garry <john.garry@huawei.com>
---
Change in v2:
- fix check on irq value
Note: I think that it would better if we could try to get the interrupt
before clk+pm enabling, so we don't need to disable later when
deferring, but this is not a fix.
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index af8beef..877fd7f 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -58,7 +58,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
struct resource resource;
struct device_node *np = ofdev->dev.of_node;
u32 clk, spd, prop;
- int ret;
+ int ret, irq;
memset(port, 0, sizeof *port);
@@ -143,21 +143,27 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
if (ret >= 0)
port->line = ret;
- port->irq = irq_of_parse_and_map(np, 0);
- if (!port->irq) {
- ret = -EPROBE_DEFER;
- goto err_unprepare;
+ irq = of_irq_get(np, 0);
+ if (irq < 0) {
+ if (irq == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err_unprepare;
+ }
+ /* IRQ support not mandatory */
+ irq = 0;
}
+ port->irq = irq;
+
info->rst = devm_reset_control_get_optional_shared(&ofdev->dev, NULL);
if (IS_ERR(info->rst)) {
ret = PTR_ERR(info->rst);
- goto err_dispose;
+ goto err_unprepare;
}
ret = reset_control_deassert(info->rst);
if (ret)
- goto err_dispose;
+ goto err_unprepare;
port->type = type;
port->uartclk = clk;
@@ -184,8 +190,6 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
port->handle_irq = fsl8250_handle_irq;
return 0;
-err_dispose:
- irq_dispose_mapping(port->irq);
err_unprepare:
clk_disable_unprepare(info->clk);
err_pmruntime:
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] serial: 8250_of: Fix for lack of interrupt support
2018-08-30 9:08 [PATCH v2] serial: 8250_of: Fix for lack of interrupt support John Garry
@ 2018-08-30 14:19 ` Rob Herring
2018-08-30 18:21 ` Alexander Sverdlin
1 sibling, 0 replies; 4+ messages in thread
From: Rob Herring @ 2018-08-30 14:19 UTC (permalink / raw)
To: John Garry
Cc: Greg Kroah-Hartman, Alexander Sverdlin, Jiri Slaby, Joel Stanley,
kurt, Masahiro Yamada, open list:SERIAL DRIVERS, linux-kernel,
Linuxarm
On Thu, Aug 30, 2018 at 4:10 AM John Garry <john.garry@huawei.com> wrote:
>
> In commit c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ"), a
> check was added for the UART driver being probed prior to the parent IRQ
> controller.
>
> Unfortunately this breaks certain boards which have no interrupt support,
> like Huawei D03.
>
> Indeed, the 8250 DT bindings state that interrupts should be supported -
> not must.
>
> To fix, switch from irq_of_parse_and_map() to of_irq_get(), which
> does relay whether the IRQ host controller domain is not ready, i.e.
> defer probe, instead of assuming it.
Good, one less user of irq_of_parse_and_map().
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] serial: 8250_of: Fix for lack of interrupt support
2018-08-30 9:08 [PATCH v2] serial: 8250_of: Fix for lack of interrupt support John Garry
2018-08-30 14:19 ` Rob Herring
@ 2018-08-30 18:21 ` Alexander Sverdlin
2018-09-17 14:55 ` John Garry
1 sibling, 1 reply; 4+ messages in thread
From: Alexander Sverdlin @ 2018-08-30 18:21 UTC (permalink / raw)
To: John Garry, gregkh
Cc: jslaby, robh, joel, kurt, yamada.masahiro, linux-serial,
linux-kernel, linuxarm
Hello John!
On 30/08/2018 11:08, John Garry wrote:
> In commit c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ"), a
> check was added for the UART driver being probed prior to the parent IRQ
> controller.
>
> Unfortunately this breaks certain boards which have no interrupt support,
> like Huawei D03.
>
> Indeed, the 8250 DT bindings state that interrupts should be supported -
> not must.
>
> To fix, switch from irq_of_parse_and_map() to of_irq_get(), which
> does relay whether the IRQ host controller domain is not ready, i.e.
> defer probe, instead of assuming it.
>
> Fixes: c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ")
> Signed-off-by: John Garry <john.garry@huawei.com>
This indeed looks like a proper way to handle both cases:
Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
Tested-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> ---
>
> Change in v2:
> - fix check on irq value
>
> Note: I think that it would better if we could try to get the interrupt
> before clk+pm enabling, so we don't need to disable later when
> deferring, but this is not a fix.
>
> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
> index af8beef..877fd7f 100644
> --- a/drivers/tty/serial/8250/8250_of.c
> +++ b/drivers/tty/serial/8250/8250_of.c
> @@ -58,7 +58,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
> struct resource resource;
> struct device_node *np = ofdev->dev.of_node;
> u32 clk, spd, prop;
> - int ret;
> + int ret, irq;
>
> memset(port, 0, sizeof *port);
>
> @@ -143,21 +143,27 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
> if (ret >= 0)
> port->line = ret;
>
> - port->irq = irq_of_parse_and_map(np, 0);
> - if (!port->irq) {
> - ret = -EPROBE_DEFER;
> - goto err_unprepare;
> + irq = of_irq_get(np, 0);
> + if (irq < 0) {
> + if (irq == -EPROBE_DEFER) {
> + ret = -EPROBE_DEFER;
> + goto err_unprepare;
> + }
> + /* IRQ support not mandatory */
> + irq = 0;
> }
>
> + port->irq = irq;
> +
> info->rst = devm_reset_control_get_optional_shared(&ofdev->dev, NULL);
> if (IS_ERR(info->rst)) {
> ret = PTR_ERR(info->rst);
> - goto err_dispose;
> + goto err_unprepare;
> }
>
> ret = reset_control_deassert(info->rst);
> if (ret)
> - goto err_dispose;
> + goto err_unprepare;
>
> port->type = type;
> port->uartclk = clk;
> @@ -184,8 +190,6 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
> port->handle_irq = fsl8250_handle_irq;
>
> return 0;
> -err_dispose:
> - irq_dispose_mapping(port->irq);
> err_unprepare:
> clk_disable_unprepare(info->clk);
> err_pmruntime:
>
--
Best regards,
Alexander Sverdlin.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] serial: 8250_of: Fix for lack of interrupt support
2018-08-30 18:21 ` Alexander Sverdlin
@ 2018-09-17 14:55 ` John Garry
0 siblings, 0 replies; 4+ messages in thread
From: John Garry @ 2018-09-17 14:55 UTC (permalink / raw)
To: gregkh
Cc: Alexander Sverdlin, jslaby, robh, joel, kurt, yamada.masahiro,
linux-serial, linux-kernel, linuxarm
On 30/08/2018 19:21, Alexander Sverdlin wrote:
> Hello John!
>
Hi Greg,
Can you kindly consider picking up this patch?
It still applies to the branches today in your tty git, but let me know
if you still rather I resend.
Thanks,
John
> On 30/08/2018 11:08, John Garry wrote:
>> In commit c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ"), a
>> check was added for the UART driver being probed prior to the parent IRQ
>> controller.
>>
>> Unfortunately this breaks certain boards which have no interrupt support,
>> like Huawei D03.
>>
>> Indeed, the 8250 DT bindings state that interrupts should be supported -
>> not must.
>>
>> To fix, switch from irq_of_parse_and_map() to of_irq_get(), which
>> does relay whether the IRQ host controller domain is not ready, i.e.
>> defer probe, instead of assuming it.
>>
>> Fixes: c58caaab3bf8 ("serial: 8250: of: Defer probe on missing IRQ")
>> Signed-off-by: John Garry <john.garry@huawei.com>
>
> This indeed looks like a proper way to handle both cases:
>
> Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> Tested-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
>
>> ---
>>
>> Change in v2:
>> - fix check on irq value
>>
>> Note: I think that it would better if we could try to get the interrupt
>> before clk+pm enabling, so we don't need to disable later when
>> deferring, but this is not a fix.
>>
>> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
>> index af8beef..877fd7f 100644
>> --- a/drivers/tty/serial/8250/8250_of.c
>> +++ b/drivers/tty/serial/8250/8250_of.c
>> @@ -58,7 +58,7 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
>> struct resource resource;
>> struct device_node *np = ofdev->dev.of_node;
>> u32 clk, spd, prop;
>> - int ret;
>> + int ret, irq;
>>
>> memset(port, 0, sizeof *port);
>>
>> @@ -143,21 +143,27 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
>> if (ret >= 0)
>> port->line = ret;
>>
>> - port->irq = irq_of_parse_and_map(np, 0);
>> - if (!port->irq) {
>> - ret = -EPROBE_DEFER;
>> - goto err_unprepare;
>> + irq = of_irq_get(np, 0);
>> + if (irq < 0) {
>> + if (irq == -EPROBE_DEFER) {
>> + ret = -EPROBE_DEFER;
>> + goto err_unprepare;
>> + }
>> + /* IRQ support not mandatory */
>> + irq = 0;
>> }
>>
>> + port->irq = irq;
>> +
>> info->rst = devm_reset_control_get_optional_shared(&ofdev->dev, NULL);
>> if (IS_ERR(info->rst)) {
>> ret = PTR_ERR(info->rst);
>> - goto err_dispose;
>> + goto err_unprepare;
>> }
>>
>> ret = reset_control_deassert(info->rst);
>> if (ret)
>> - goto err_dispose;
>> + goto err_unprepare;
>>
>> port->type = type;
>> port->uartclk = clk;
>> @@ -184,8 +190,6 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
>> port->handle_irq = fsl8250_handle_irq;
>>
>> return 0;
>> -err_dispose:
>> - irq_dispose_mapping(port->irq);
>> err_unprepare:
>> clk_disable_unprepare(info->clk);
>> err_pmruntime:
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-17 14:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-30 9:08 [PATCH v2] serial: 8250_of: Fix for lack of interrupt support John Garry
2018-08-30 14:19 ` Rob Herring
2018-08-30 18:21 ` Alexander Sverdlin
2018-09-17 14:55 ` John Garry
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).