Skip to contentSkip to conversation

Small fixes for Renesas R-Car CAN driver

PATCH In review Sergei Shtylyov 3 participants 8 messages Jun 20, 2015

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

cover Small fixes for Renesas R-Car CAN driver

Hello.

Here's the set of 3 patches against Marc Kleine-Budde's 'linux-can.git'
repo; they are small fixes for the Renesas R-Car CAN driver.

rcar_can: fix IRQ check
rcar_can: print signed IRQ #
rcar_can: fix typo in error message

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

#1 lore raw

PATCH 1/3 rcar_can: fix IRQ check

rcar_can_probe() regards 0 as a wrong IRQ #, despite platform_get_irq() that it calls returns negative error code in that case. This leads to the following being printed to the console when attempting to open the device:

error requesting interrupt fffffffa

because rcar_can_open() calls request_irq() with a negative IRQ #, and that function naturally fails with -EINVAL.

Check for the negative error codes instead and propagate them upstream instead of just returning -ENODEV.

Fixes fd1159318e55 ("can: add Renesas R-Car CAN driver")
---
 drivers/net/can/rcar_can.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-can/drivers/net/can/rcar_can.c =================================--- linux-can.orig/drivers/net/can/rcar_can.c +++ linux-can/drivers/net/can/rcar_can.c
@@ -758,8 +758,9 @@ static int rcar_can_probe(struct platfor
}

irq = platform_get_irq(pdev, 0);
- if (!irq) {
+ if (irq < 0) {
dev_err(&pdev->dev, "No IRQ resource\n");
+ err = irq;
goto fail;
}

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

#2 lore raw

PATCH 2/3 rcar_can: print signed IRQ #

In reply to Sergei Shtylyov

Printing IRQ # using "%x" and "%u" unsigned formats isn't quite correct as 'ndev->irq' is of type *int*, so the "%d" format needs to be used instead.

While fixing this, beautify the dev_info() message in rcar_can_probe() a bit.

Fixes fd1159318e55 ("can: add Renesas R-Car CAN driver")
---
 drivers/net/can/rcar_can.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-can/drivers/net/can/rcar_can.c =================================--- linux-can.orig/drivers/net/can/rcar_can.c +++ linux-can/drivers/net/can/rcar_can.c
@@ -526,7 +526,7 @@ static int rcar_can_open(struct net_devi
napi_enable(&priv->napi);
err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev); if (err) {
- netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
+ netdev_err(ndev, "error requesting interrupt %d\n", ndev->irq);
goto out_close;
}
can_led_event(ndev, CAN_LED_EVENT_OPEN);
@@ -824,7 +824,7 @@ static int rcar_can_probe(struct platfor

devm_can_led_init(ndev);

- dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n",
+ dev_info(&pdev->dev, "device registered (regs @ %p, IRQ%d)\n",
priv->regs, ndev->irq);

return 0;

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

#3 lore raw

Re: [PATCH 2/3] rcar_can: print signed IRQ #

Hi Sergei,

Quoted 4 lines · Sergei Shtylyov

Printing IRQ # using "%x" and "%u" unsigned formats isn't quite correct as 'ndev->irq' is of type *int*, so the "%d" format needs to be used instead.

While fixing this, beautify the dev_info() message in rcar_can_probe() a bit.

If you change the message, why don't you make it consistent ("interrupt" vs. "IRQ")?

Quoted 21 lines
Fixes fd1159318e55 ("can: add Renesas R-Car CAN driver")
---
 drivers/net/can/rcar_can.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-can/drivers/net/can/rcar_can.c
===================================================================

drivers/net/can/rcar_can.c +1 -1
@@ -526,7 +526,7 @@ static int rcar_can_open(struct net_devi
        napi_enable(&priv->napi);
        err = request_irq(ndev->irq, rcar_can_interrupt, 0, ndev->name, ndev);
        if (err) {
-               netdev_err(ndev, "error requesting interrupt %x\n", ndev->irq);
+               netdev_err(ndev, "error requesting interrupt %d\n", ndev->irq);
                goto out_close;
        }
        can_led_event(ndev, CAN_LED_EVENT_OPEN);

drivers/net/can/rcar_can.c static int rcar_can_probe(struct platfor · line 828

Quoted 6 lines
drivers/net/can/rcar_can.c +1 -1
@@ -824,7 +824,7 @@ static int rcar_can_probe(struct platfor
 
        devm_can_led_init(ndev);
 
-       dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n",
+       dev_info(&pdev->dev, "device registered (regs @ %p, IRQ%d)\n",
                 priv->regs, ndev->irq);

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#4 lore raw

Re: [PATCH 2/3] rcar_can: print signed IRQ #

Hello.

Quoted 7 lines · Geert Uytterhoeven
Quoted 4 lines

Printing IRQ # using "%x" and "%u" unsigned formats isn't quite correct as 'ndev->irq' is of type *int*, so the "%d" format needs to be used instead.

While fixing this, beautify the dev_info() message in rcar_can_probe() a bit.

If you change the message, why don't you make it consistent ("interrupt" vs. "IRQ")?

I decided to change the message in a follow-up patch (posted afterwards).

Quoted 2 lines
Quoted 2 lines
Fixes fd1159318e55 ("can: add Renesas R-Car CAN driver")

[...]

Quoted 2 lines

Gr{oetje,eeting}s,
Geert

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

#5 lore raw

Re: [PATCH 2/3] rcar_can: print signed IRQ #

Quoted 7 lines · Sergei Shtylyov

Hello.

Quoted 2 lines · Geert Uytterhoeven
Quoted 2 lines

Printing IRQ # using "%x" and "%u" unsigned formats isn't quite correct as 'ndev->irq' is of type *int*, so the "%d" format needs to be used instead.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

While fixing this, beautify the dev_info() message in rcar_can_probe() a bit.

If you change the message, why don't you make it consistent ("interrupt" vs. "IRQ")?

I decided to change the message in a follow-up patch (posted afterwards).

Please squash you patches, so that you don't modify code (or error messages) that you've added in a previous patch.

Marc

Signature · 4 lines
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
#6 lore raw

Re: [PATCH 2/3] rcar_can: print signed IRQ #

Hello.

Quoted 5 lines · Marc Kleine-Budde
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Printing IRQ # using "%x" and "%u" unsigned formats isn't quite correct as 'ndev->irq' is of type *int*, so the "%d" format needs to be used instead.

While fixing this, beautify the dev_info() message in rcar_can_probe() a bit.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

If you change the message, why don't you make it consistent ("interrupt" vs. "IRQ")?

I decided to change the message in a follow-up patch (posted afterwards).

Please squash you patches, so that you don't modify code (or error messages) that you've added in a previous patch.

I didn't add any messages.

Quoted 1 line

Marc

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

#7 lore raw

PATCH 3/3 rcar_can: fix typo in error message

In reply to Sergei Shtylyov

Fix typo in the first error message printed by rcar_can_open().

Based on the original patch by Vladimir Barinov.

Fixes 862e2b6af941 ("can: rcar_can: support all input clocks")
---
 drivers/net/can/rcar_can.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/can/rcar_can.c +2 -1
Index: linux-can/drivers/net/can/rcar_can.c
===================================================================
@@ -508,7 +508,8 @@ static int rcar_can_open(struct net_devi
 
 	err = clk_prepare_enable(priv->clk);
 	if (err) {
-		netdev_err(ndev, "failed to enable periperal clock, error %d\n",
+		netdev_err(ndev,
+			   "failed to enable peripheral clock, error %d\n",
 			   err);
 		goto out;
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in