Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1327174

Re: [PATCH] net: ethernet: davicom: fix devicetree irq resource

From Robert Jarzmik <robert.jarzmik@free.fr>
Newsgroups linux.kernel
Subject Re: [PATCH] net: ethernet: davicom: fix devicetree irq resource
Date 2016-02-04 21:50 +0100
Message-ID <qYz0J-70Q-3@gated-at.bofh.it> (permalink)
References <qYdtf-7qy-1@gated-at.bofh.it> <qYvJw-4Y7-19@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes:

> Hello.
>
>    Your patch summary prefixes are too verbose, it was enough to say only
> "dm9000: ".
Well, I don't agree here. The subsystem should be fully specified, at least this
is something I require in pxa, something that is also required in sound/*, etc
... If David doesn't object, I'll keep it that way. As it's his tree, his
decision in the end, so let's have him decide.

>> -	/* If there is no IRQ type specified, default to something that
>> -	 * may work, and tell the user that this is a problem */
>> -
>> -	if (irqflags == IRQF_TRIGGER_NONE)
>> -		irqflags = irq_get_trigger_type(dev->irq);
>> -
>> -	if (irqflags == IRQF_TRIGGER_NONE)
>> +	/* If there is no IRQ type specified, tell the user that this is a
>> +	 * problem */
>
>    The networking code formats comments this way:
>
> /* foo
>  * bar
>  */
May I know where this is documented ?
I'm asking because I didn't find it, because I parsed drivers/net/*.c files, and
the standard kernel comment style was there, ie:
/*
 * foo
 * bar
 */

I was reusing the previous comment style, but I will change it for the standard
kernel style if you wish.

>>
>> +	ndev->irq = platform_get_irq(pdev, 0);
>> +	if (ndev->irq <= 0) {
>
>    I don't recommend checking for 0 and returning early in this case -- 
> you'll signal a probe success this way. Either ignore 0 or return -E<smth>
> in this case. Unfortunately, platform_get_irq() is so sloppily coded now that it
> *can* return 0 on error. :-(
Ah we had that discussion not very long ago, didn't we ? :)
And I think I'll reuse the "if (ndev->irq < 0) {" solution to be consistent with
myself.

Thanks for the review.

-- 
Robert

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] net: ethernet: davicom: fix devicetree irq resource Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-03 22:50 +0100
  Re: [PATCH] net: ethernet: davicom: fix devicetree irq resource Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-02-04 18:20 +0100
    Re: [PATCH] net: ethernet: davicom: fix devicetree irq resource Robert Jarzmik <robert.jarzmik@free.fr> - 2016-02-04 21:50 +0100
      Re: [PATCH] net: ethernet: davicom: fix devicetree irq resource Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-02-04 22:00 +0100

csiph-web