[PATCH 0/3] Small fixes for Renesas R-Car CAN driver

[PATCH 0/3] Small fixes for Renesas R-Car CAN driver

[Sergei Shtylyov]

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-can" in

[PATCH 1/3] rcar_can: fix IRQ check

[Sergei Shtylyov]

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")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 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-can" in

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

[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")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 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-can" in

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

[Geert Uytterhoeven]

Hi Sergei,

On Sat, Jun 20, 2015 at 2:33 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> 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")?

> Fixes: fd1159318e55 ("can: add Renesas R-Car CAN driver")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
>  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);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

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

[Sergei Shtylyov]

Hello.

On 6/20/2015 3:03 PM, Geert Uytterhoeven wrote:

>> 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).

>> Fixes: fd1159318e55 ("can: add Renesas R-Car CAN driver")
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

[...]

> Gr{oetje,eeting}s,
>                          Geert

WBR, Sergei

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

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

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 954 bytes --]

On 06/20/2015 04:38 PM, Sergei Shtylyov wrote:
> Hello.
> 
> On 6/20/2015 3:03 PM, Geert Uytterhoeven wrote:
> 
>>> 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).

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

Marc

-- 
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   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

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

[Sergei Shtylyov]

Hello.

On 6/20/2015 7:02 PM, Marc Kleine-Budde wrote:

>>>> 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).

> 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.

> Marc

WBR, Sergei

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

[PATCH 3/3] rcar_can: fix typo in error message

[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")
Reported-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
 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
@@ -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