* [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' @ 2018-09-20 1:58 YueHaibing 2018-09-20 7:19 ` Uwe Kleine-König [not found] ` <3b23675a-f537-868f-2432-b7f8a0c8c466@suse.cz> 0 siblings, 2 replies; 8+ messages in thread From: YueHaibing @ 2018-09-20 1:58 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: YueHaibing, linux-serial, kernel-janitors, linux-kernel Fixes gcc '-Wunused-but-set-variable' warning: drivers/tty/serial/imx.c: In function 'imx_uart_probe': drivers/tty/serial/imx.c:2198:20: warning: variable 'rtsirq' set but not used [-Wunused-but-set-variable] Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- drivers/tty/serial/imx.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 4341589..1df7d23 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2195,7 +2195,7 @@ static int imx_uart_probe(struct platform_device *pdev) int ret = 0; u32 ucr1; struct resource *res; - int txirq, rxirq, rtsirq; + int txirq, rxirq; sport = devm_kzalloc(&pdev->dev, sizeof(*sport), GFP_KERNEL); if (!sport) @@ -2220,7 +2220,6 @@ static int imx_uart_probe(struct platform_device *pdev) rxirq = platform_get_irq(pdev, 0); txirq = platform_get_irq(pdev, 1); - rtsirq = platform_get_irq(pdev, 2); sport->port.dev = &pdev->dev; sport->port.mapbase = res->start; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' 2018-09-20 1:58 [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' YueHaibing @ 2018-09-20 7:19 ` Uwe Kleine-König 2018-09-20 12:11 ` [PATCH] serial: imx: restore handshaking irq for imx1 Uwe Kleine-König [not found] ` <3b23675a-f537-868f-2432-b7f8a0c8c466@suse.cz> 1 sibling, 1 reply; 8+ messages in thread From: Uwe Kleine-König @ 2018-09-20 7:19 UTC (permalink / raw) To: YueHaibing Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, kernel-janitors, linux-kernel On Thu, Sep 20, 2018 at 01:58:45AM +0000, YueHaibing wrote: > Fixes gcc '-Wunused-but-set-variable' warning: > > drivers/tty/serial/imx.c: In function 'imx_uart_probe': > drivers/tty/serial/imx.c:2198:20: warning: > variable 'rtsirq' set but not used [-Wunused-but-set-variable] > > Signed-off-by: YueHaibing <yuehaibing@huawei.com> Wow, this variable is write-only since afe9cbb1a6ad ("serial: imx: drop support for IRDA") which is over three years old. The last hunk of this patch is wrong however, which means that nobody uses handshaking on imx1 with a kernel newer than 4.1-rc1. (Well, or they fixed it and didn't made the effort to tell.) I suggest to break rx and tx on imx1, too, and if nobody reports a regression within the next three years, we rip out imx1 support completely. :-) Otherwise we need: diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 4e853570ea80..554a69db1bca 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2350,6 +2350,14 @@ static int imx_uart_probe(struct platform_device *pdev) ret); return ret; } + + ret = devm_request_irq(&pdev->dev, rtsirq, imx_uart_rtsint, 0, + dev_name(&pdev->dev), sport); + if (ret) { + dev_err(&pdev->dev, "failed to request rts irq: %d\n", + ret); + return ret; + } } else { ret = devm_request_irq(&pdev->dev, rxirq, imx_uart_int, 0, dev_name(&pdev->dev), sport); Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] serial: imx: restore handshaking irq for imx1 2018-09-20 7:19 ` Uwe Kleine-König @ 2018-09-20 12:11 ` Uwe Kleine-König 2018-09-20 12:17 ` Leonard Crestez 2018-09-21 1:18 ` Andy Duan 0 siblings, 2 replies; 8+ messages in thread From: Uwe Kleine-König @ 2018-09-20 12:11 UTC (permalink / raw) To: Greg Kroah-Hartman, Jiri Slaby Cc: YueHaibing, linux-serial, kernel, kernel-janitors, linux-kernel, Leonard Crestez, Andy Duan Back in 2015 when irda was dropped from the driver imx1 was broken. This change reintroduces the support for the third interrupt of the UART. Fixes: afe9cbb1a6ad ("serial: imx: drop support for IRDA") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/tty/serial/imx.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index 4e853570ea80..554a69db1bca 100644 --- a/drivers/tty/serial/imx.c +++ b/drivers/tty/serial/imx.c @@ -2350,6 +2350,14 @@ static int imx_uart_probe(struct platform_device *pdev) ret); return ret; } + + ret = devm_request_irq(&pdev->dev, rtsirq, imx_uart_rtsint, 0, + dev_name(&pdev->dev), sport); + if (ret) { + dev_err(&pdev->dev, "failed to request rts irq: %d\n", + ret); + return ret; + } } else { ret = devm_request_irq(&pdev->dev, rxirq, imx_uart_int, 0, dev_name(&pdev->dev), sport); -- 2.18.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] serial: imx: restore handshaking irq for imx1 2018-09-20 12:11 ` [PATCH] serial: imx: restore handshaking irq for imx1 Uwe Kleine-König @ 2018-09-20 12:17 ` Leonard Crestez 2018-09-21 1:18 ` Andy Duan 1 sibling, 0 replies; 8+ messages in thread From: Leonard Crestez @ 2018-09-20 12:17 UTC (permalink / raw) To: jslaby, u.kleine-koenig, gregkh Cc: yuehaibing, linux-serial, linux-kernel, kernel, Andy Duan, kernel-janitors On Thu, 2018-09-20 at 14:11 +0200, Uwe Kleine-König wrote: > Back in 2015 when irda was dropped from the driver imx1 was broken. This > change reintroduces the support for the third interrupt of the UART. > > Fixes: afe9cbb1a6ad ("serial: imx: drop support for IRDA") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Reviewed-by: Leonard Crestez <leonard.crestez@nxp.com> I'm not sure if anyone has imx1 hardware handy can is willing to test this CTS/RTS but the fix is pretty obviously correct. ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] serial: imx: restore handshaking irq for imx1 2018-09-20 12:11 ` [PATCH] serial: imx: restore handshaking irq for imx1 Uwe Kleine-König 2018-09-20 12:17 ` Leonard Crestez @ 2018-09-21 1:18 ` Andy Duan 1 sibling, 0 replies; 8+ messages in thread From: Andy Duan @ 2018-09-21 1:18 UTC (permalink / raw) To: Uwe Kleine-König, Greg Kroah-Hartman, Jiri Slaby Cc: YueHaibing, linux-serial, kernel, kernel-janitors, linux-kernel, Leonard Crestez From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Sent: 2018年9月20日 20:11 > Back in 2015 when irda was dropped from the driver imx1 was broken. > This change reintroduces the support for the third interrupt of the UART. > > Fixes: afe9cbb1a6ad ("serial: imx: drop support for IRDA") > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/tty/serial/imx.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c index > 4e853570ea80..554a69db1bca 100644 > --- a/drivers/tty/serial/imx.c > +++ b/drivers/tty/serial/imx.c > @@ -2350,6 +2350,14 @@ static int imx_uart_probe(struct > platform_device *pdev) > ret); > return ret; > } > + > + ret = devm_request_irq(&pdev->dev, rtsirq, imx_uart_rtsint, 0, > + dev_name(&pdev->dev), sport); > + if (ret) { > + dev_err(&pdev->dev, "failed to request rts irq: %d\n", > + ret); > + return ret; > + } > } else { > ret = devm_request_irq(&pdev->dev, rxirq, imx_uart_int, 0, > dev_name(&pdev->dev), sport); > -- > 2.18.0 Reviewed-by: Fugang Duan <fugang.duan@nxp.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <3b23675a-f537-868f-2432-b7f8a0c8c466@suse.cz>]
* Re: [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' [not found] ` <3b23675a-f537-868f-2432-b7f8a0c8c466@suse.cz> @ 2018-09-20 8:50 ` Leonard Crestez 2018-09-20 9:41 ` Andy Duan 0 siblings, 1 reply; 8+ messages in thread From: Leonard Crestez @ 2018-09-20 8:50 UTC (permalink / raw) To: yuehaibing, u.kleine-koenig, jslaby Cc: linux-serial, linux-kernel, dl-linux-imx, gregkh, Andy Duan, kernel-janitors On Thu, 2018-09-20 at 08:45 +0200, Jiri Slaby wrote: > On 09/20/2018, 03:58 AM, YueHaibing wrote: > > Fixes gcc '-Wunused-but-set-variable' warning: > > > > drivers/tty/serial/imx.c: In function 'imx_uart_probe': > > drivers/tty/serial/imx.c:2198:20: warning: > > variable 'rtsirq' set but not used [-Wunused-but-set-variable] > > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > @@ -2220,7 +2220,6 @@ static int imx_uart_probe(struct platform_device *pdev) > > > > rxirq = platform_get_irq(pdev, 0); > > txirq = platform_get_irq(pdev, 1); > > - rtsirq = platform_get_irq(pdev, 2); > > I am not sure this is correct. platform_get_irq has side effects (like > enabling the IRQ). Are you sure this won't change the behaviour (this is > question mostly to IMX fellows)? As far as I can tell there was a request_irq call for rtsirq which was removed by mistake in commit afe9cbb1a6ad ("serial: imx: drop support for IRDA"): - /* do not use RTS IRQ on IrDA */ - if (!USE_IRDA(sport)) { - ret = devm_request_irq(&pdev->dev, rtsirq, - imx_rtsint, 0, - dev_name(&pdev->dev), sport); - if (ret) - return ret; - } This should have just removed the IRDA check and request rtsirq unconditionally. Nobody noticed this by testing RTS on imx1, this is an old chip and later variants have a single combined irq. The correct fix for the warning would be to restore that request_irq. -- Regards, Leonard ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' 2018-09-20 8:50 ` [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' Leonard Crestez @ 2018-09-20 9:41 ` Andy Duan 2018-09-20 12:04 ` YueHaibing 0 siblings, 1 reply; 8+ messages in thread From: Andy Duan @ 2018-09-20 9:41 UTC (permalink / raw) To: Leonard Crestez, yuehaibing, u.kleine-koenig, jslaby Cc: linux-serial, linux-kernel, A.s. Dong, Abel Vesa, Anson Huang, Bough Chen, Cosmin Samoila, Daniel Baluta, Han Xu, Jacky Bai, Jun Li, Leo Zhang, Leonard Crestez, Peng Fan, Peter Chen, Ranjani Vaidyanathan, Robert Chiras, Robin Gong, Shenwei Wang, Viorel Suman, Zening Wang, gregkh, kernel-janitors From: Leonard Crestez Sent: 2018年9月20日 16:51 > On Thu, 2018-09-20 at 08:45 +0200, Jiri Slaby wrote: > > On 09/20/2018, 03:58 AM, YueHaibing wrote: > > > Fixes gcc '-Wunused-but-set-variable' warning: > > > > > > drivers/tty/serial/imx.c: In function 'imx_uart_probe': > > > drivers/tty/serial/imx.c:2198:20: warning: > > > variable 'rtsirq' set but not used [-Wunused-but-set-variable] > > > > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c @@ > > > -2220,7 +2220,6 @@ static int imx_uart_probe(struct > platform_device > > > *pdev) > > > > > > rxirq = platform_get_irq(pdev, 0); > > > txirq = platform_get_irq(pdev, 1); > > > - rtsirq = platform_get_irq(pdev, 2); > > > > I am not sure this is correct. platform_get_irq has side effects (like > > enabling the IRQ). Are you sure this won't change the behaviour (this > > is question mostly to IMX fellows)? > > As far as I can tell there was a request_irq call for rtsirq which was > removed by mistake in commit afe9cbb1a6ad ("serial: imx: drop support > for IRDA"): > > - /* do not use RTS IRQ on IrDA */ > - if (!USE_IRDA(sport)) { > - ret = devm_request_irq(&pdev->dev, rtsirq, > - imx_rtsint, 0, > - > dev_name(&pdev->dev), sport); > - if (ret) > - return ret; > - } > > This should have just removed the IRDA check and request rtsirq > unconditionally. Nobody noticed this by testing RTS on imx1, this is an old > chip and later variants have a single combined irq. > > The correct fix for the warning would be to restore that request_irq. > > -- > Regards, > Leonard Yes, your explain is very correct! Thanks for your comment. We should restore rtsirq request that for i.MX1. Regards, Andy Duan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' 2018-09-20 9:41 ` Andy Duan @ 2018-09-20 12:04 ` YueHaibing 0 siblings, 0 replies; 8+ messages in thread From: YueHaibing @ 2018-09-20 12:04 UTC (permalink / raw) To: Andy Duan, Leonard Crestez, u.kleine-koenig, jslaby Cc: linux-serial, linux-kernel, A.s. Dong, Abel Vesa, Anson Huang, Bough Chen, Cosmin Samoila, Daniel Baluta, Han Xu, Jacky Bai, Jun Li, Leo Zhang, Peng Fan, Peter Chen, Ranjani Vaidyanathan, Robert Chiras, Robin Gong, Shenwei Wang, Viorel Suman, Zening Wang, gregkh, kernel-janitors On 2018/9/20 17:41, Andy Duan wrote: > From: Leonard Crestez Sent: 2018年9月20日 16:51 >> On Thu, 2018-09-20 at 08:45 +0200, Jiri Slaby wrote: >>> On 09/20/2018, 03:58 AM, YueHaibing wrote: >>>> Fixes gcc '-Wunused-but-set-variable' warning: >>>> >>>> drivers/tty/serial/imx.c: In function 'imx_uart_probe': >>>> drivers/tty/serial/imx.c:2198:20: warning: >>>> variable 'rtsirq' set but not used [-Wunused-but-set-variable] >>>> >>>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c @@ >>>> -2220,7 +2220,6 @@ static int imx_uart_probe(struct >> platform_device >>>> *pdev) >>>> >>>> rxirq = platform_get_irq(pdev, 0); >>>> txirq = platform_get_irq(pdev, 1); >>>> - rtsirq = platform_get_irq(pdev, 2); >>> >>> I am not sure this is correct. platform_get_irq has side effects (like >>> enabling the IRQ). Are you sure this won't change the behaviour (this >>> is question mostly to IMX fellows)? >> >> As far as I can tell there was a request_irq call for rtsirq which was >> removed by mistake in commit afe9cbb1a6ad ("serial: imx: drop support >> for IRDA"): >> >> - /* do not use RTS IRQ on IrDA */ >> - if (!USE_IRDA(sport)) { >> - ret = devm_request_irq(&pdev->dev, rtsirq, >> - imx_rtsint, 0, >> - >> dev_name(&pdev->dev), sport); >> - if (ret) >> - return ret; >> - } >> >> This should have just removed the IRDA check and request rtsirq >> unconditionally. Nobody noticed this by testing RTS on imx1, this is an old >> chip and later variants have a single combined irq. >> >> The correct fix for the warning would be to restore that request_irq. >> >> -- >> Regards, >> Leonard > > Yes, your explain is very correct! Thanks for your comment. > We should restore rtsirq request that for i.MX1. Ok, I will post a new fix patch as Leonard and suggested. > > Regards, > Andy Duan > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-09-21 1:18 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-20 1:58 [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' YueHaibing 2018-09-20 7:19 ` Uwe Kleine-König 2018-09-20 12:11 ` [PATCH] serial: imx: restore handshaking irq for imx1 Uwe Kleine-König 2018-09-20 12:17 ` Leonard Crestez 2018-09-21 1:18 ` Andy Duan [not found] ` <3b23675a-f537-868f-2432-b7f8a0c8c466@suse.cz> 2018-09-20 8:50 ` [PATCH -next] serial: imx: remove set but not used variable 'rtsirq' Leonard Crestez 2018-09-20 9:41 ` Andy Duan 2018-09-20 12:04 ` YueHaibing
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).