linux-kernel - Re: [PATCH v3 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 3 Dec 2014 20:08:11 +0800
From:	Lyra Zhang <zhang.lyra@...il.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Arnd Bergmann <arnd@...db.de>, Mark Rutland <mark.rutland@....com>,
	"gnomes@...rguk.ukuu.org.uk" <gnomes@...rguk.ukuu.org.uk>,
	Heiko Stübner <heiko@...ech.de>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	"andrew@...n.ch" <andrew@...n.ch>,
	"linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
	"jslaby@...e.cz" <jslaby@...e.cz>,
	Ramkumar Ramachandra <artagnon@...il.com>,
	"lanqing.liu@...eadtrum.com" <lanqing.liu@...eadtrum.com>,
	Wei Qiao (乔伟) <wei.qiao@...eadtrum.com>,
	Jonathan Corbet <corbet@....net>,
	Chunyan Zhang <chunyan.zhang@...eadtrum.com>,
	"zhizhou.zhang" <zhizhou.zhang@...eadtrum.com>,
	"geng.ren@...eadtrum.com" <geng.ren@...eadtrum.com>,
	Murali Karicheri <m-karicheri2@...com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	Grant Likely <grant.likely@...aro.org>,
	Orson Zhai <orsonzhai@...il.com>,
	"florian.vaussard@...l.ch" <florian.vaussard@...l.ch>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"jason@...edaemon.net" <jason@...edaemon.net>,
	Pawel Moll <pawel.moll@....com>,
	"ijc+devicetree@...lion.org.uk" <ijc+devicetree@...lion.org.uk>,
	Hayato Suzuki <hytszk@...il.com>,
	"rrichter@...ium.com" <rrichter@...ium.com>,
	Mark Brown <broonie@...nel.org>,
	"sprdlinux@...elists.org" <sprdlinux@...elists.org>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	Kumar Gala <galak@...eaurora.org>
Subject: Re: [PATCH v3 5/5] tty/serial: Add Spreadtrum sc9836-uart driver support

2014-12-03 18:11 GMT+08:00 Russell King - ARM Linux <linux@....linux.org.uk>:
> On Wed, Dec 03, 2014 at 10:50:17AM +0100, Arnd Bergmann wrote:
>> On Wednesday 03 December 2014 17:17:13 Lyra Zhang wrote:
>>
>> > 2014-11-27 2:29 GMT+08:00 Murali Karicheri <m-karicheri2@...com>:
>> > > How about sorting this includes? asm/irq.h go first followed linux/ in alphabatical order?
>> >
>> > There are a few compile warnings if I moved asm/irq.h to the top of
>> > all included files.
>>
>> The order that Murali meant is
>>
>> - first all linux/*.h headers, sorted alphabetically
>> - then all asm/*.h headers, again sorted alphabetically
>>
>> This is the recommended style in general.
>>
>> > Warning details are below:
>> >
>> > In file included from drivers/tty/serial/sprd_serial.c:14:0:
>> > ./arch/arm64/include/asm/irq.h:6:39: warning: ‘struct pt_regs’
>> > declared inside parameter list [enabled by default]
>> >  extern void (*handle_arch_irq)(struct pt_regs *);
>> >                                        ^
>> > ./arch/arm64/include/asm/irq.h:6:39: warning: its scope is only this
>> > definition or declaration, which is probably not what you want
>> > [enabled by default]
>> > ./arch/arm64/include/asm/irq.h:8:54: warning: ‘struct pt_regs’
>> > declared inside parameter list [enabled by default]
>> >  extern void set_handle_irq(void (*handle_irq)(struct pt_regs *));
>> >
>>
>> I would consider this a (minor) bug in asm/irq.h. If you don't mind,
>> please submit a patch to add a line with 'struct pt_regs;' to
>> asm/irq.h.
>
> A better question is: why is a modern driver using asm/irq.h in the
> first place.
>
> We used to include that file when platforms defined IRQ numbers
> statically, but modern platforms don't do that, so it shouldn't be
> required anymore.
>

OK, I see. I'll remove it in v4.

I'm very sorry that I didn't check it carefully enough and troubled
many people for this problem.
This file included the asm/irq.h before I revised it for upstreaming,
I also could not find this file was no longer needed to include.
Thank you for your reminder, I'll avoid making the same mistake, and
I'll also share my experience with my colleagues to keep them from
having this mistake in the future.

Best regards,
Chunyan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/