2013-06-08 17:31:51

by Graeme Smecher

[permalink] [raw]
Subject: [PATCH] microblaze: Don't mark arch_kgdb_ops as const.

Other architectures don't do it, and it conflicts with the extern'd definition
in include/linux/kgdb.h.

Signed-off-by: Graeme Smecher <[email protected]>
CC: Michal Simek <[email protected]>
CC: [email protected]
---
arch/microblaze/kernel/kgdb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/microblaze/kernel/kgdb.c b/arch/microblaze/kernel/kgdb.c
index 8adc9244..09a5e82 100644
--- a/arch/microblaze/kernel/kgdb.c
+++ b/arch/microblaze/kernel/kgdb.c
@@ -141,7 +141,7 @@ void kgdb_arch_exit(void)
/*
* Global data
*/
-const struct kgdb_arch arch_kgdb_ops = {
+struct kgdb_arch arch_kgdb_ops = {
#ifdef __MICROBLAZEEL__
.gdb_bpt_instr = {0x18, 0x00, 0x0c, 0xba}, /* brki r16, 0x18 */
#else
--
1.7.7.3


2013-06-08 21:01:10

by Graeme Smecher

[permalink] [raw]
Subject: Re: [PATCH] microblaze: Don't mark arch_kgdb_ops as const.

Hi all,

On 08/06/13 09:52 AM, Graeme Smecher wrote:
> Other architectures don't do it, and it conflicts with the extern'd definition
> in include/linux/kgdb.h.
>
> Signed-off-by: Graeme Smecher<[email protected]>
> CC: Michal Simek<[email protected]>
> CC:[email protected]
> ---
> arch/microblaze/kernel/kgdb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/microblaze/kernel/kgdb.c b/arch/microblaze/kernel/kgdb.c
> index 8adc9244..09a5e82 100644
> --- a/arch/microblaze/kernel/kgdb.c
> +++ b/arch/microblaze/kernel/kgdb.c
> @@ -141,7 +141,7 @@ void kgdb_arch_exit(void)
> /*
> * Global data
> */
> -const struct kgdb_arch arch_kgdb_ops = {
> +struct kgdb_arch arch_kgdb_ops = {
> #ifdef __MICROBLAZEEL__
> .gdb_bpt_instr = {0x18, 0x00, 0x0c, 0xba}, /* brki r16, 0x18 */
> #else

It's worth noting that this patch is flagged by checkpatch:

WARNING: struct kgdb_arch should normally be const
#25: FILE: arch/microblaze/kernel/kgdb.c:144:
+struct kgdb_arch arch_kgdb_ops = {

total: 0 errors, 1 warnings, 8 lines checked


However, without it, my kernel fails to compile:

CC arch/microblaze/kernel/kgdb.o
arch/microblaze/kernel/kgdb.c:144:24: error: conflicting type
qualifiers for 'arch_kgdb_ops'
include/linux/kgdb.h:284:26: note: previous declaration of
'arch_kgdb_ops' was here
make[1]: *** [arch/microblaze/kernel/kgdb.o] Error 1
make: *** [arch/microblaze/kernel] Error 2


The checkpatch test was a response to this (rejected) patch series:

http://lkml.indiana.edu/hypermail/linux/kernel/0912.1/02659.html

I'm the lucky victim, here, since I'm using gcc 4.6.4 (which notices the
conflicting qualifiers), while other Microblaze users are still working
with gcc 4.1.2 (which didn't care.) All other architectures (including
mn10300, arc, and hexagon, which were accepted after the checkpatch
commit) ignore the checkpatch rule and declare arch_kgdb_ops non-const.

In any case, I wanted to point out (a) that the patch fails checkpatch,
(b) that I'm submitting it anyway, and (c) that I'm utterly ambivalent
about how the struct is declared, provided I can compile working kernels
on Microblaze.

cheers,
Graeme

2013-06-23 21:17:42

by Graeme Smecher

[permalink] [raw]
Subject: Re: [PATCH] microblaze: Don't mark arch_kgdb_ops as const.

Hi Michal,

On 08/06/13 01:58 PM, Graeme Smecher wrote:
> Hi all,
>
> On 08/06/13 09:52 AM, Graeme Smecher wrote:
>> Other architectures don't do it, and it conflicts with the extern'd
>> definition
>> in include/linux/kgdb.h.
>>
>> Signed-off-by: Graeme Smecher<[email protected]>
>> CC: Michal Simek<[email protected]>
>> CC:[email protected]
>> ---
>> arch/microblaze/kernel/kgdb.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/microblaze/kernel/kgdb.c
>> b/arch/microblaze/kernel/kgdb.c
>> index 8adc9244..09a5e82 100644
>> --- a/arch/microblaze/kernel/kgdb.c
>> +++ b/arch/microblaze/kernel/kgdb.c
>> @@ -141,7 +141,7 @@ void kgdb_arch_exit(void)
>> /*
>> * Global data
>> */
>> -const struct kgdb_arch arch_kgdb_ops = {
>> +struct kgdb_arch arch_kgdb_ops = {
>> #ifdef __MICROBLAZEEL__
>> .gdb_bpt_instr = {0x18, 0x00, 0x0c, 0xba}, /* brki r16, 0x18 */
>> #else
>
> It's worth noting that this patch is flagged by checkpatch:
>
> WARNING: struct kgdb_arch should normally be const
> #25: FILE: arch/microblaze/kernel/kgdb.c:144:
> +struct kgdb_arch arch_kgdb_ops = {
>
> total: 0 errors, 1 warnings, 8 lines checked
>
>
> However, without it, my kernel fails to compile:
>
> CC arch/microblaze/kernel/kgdb.o
> arch/microblaze/kernel/kgdb.c:144:24: error: conflicting type
> qualifiers for 'arch_kgdb_ops'
> include/linux/kgdb.h:284:26: note: previous declaration of
> 'arch_kgdb_ops' was here
> make[1]: *** [arch/microblaze/kernel/kgdb.o] Error 1
> make: *** [arch/microblaze/kernel] Error 2
>
>
> The checkpatch test was a response to this (rejected) patch series:
>
> http://lkml.indiana.edu/hypermail/linux/kernel/0912.1/02659.html
>
> I'm the lucky victim, here, since I'm using gcc 4.6.4 (which notices
> the conflicting qualifiers), while other Microblaze users are still
> working with gcc 4.1.2 (which didn't care.) All other architectures
> (including mn10300, arc, and hexagon, which were accepted after the
> checkpatch commit) ignore the checkpatch rule and declare
> arch_kgdb_ops non-const.
>
> In any case, I wanted to point out (a) that the patch fails
> checkpatch, (b) that I'm submitting it anyway, and (c) that I'm
> utterly ambivalent about how the struct is declared, provided I can
> compile working kernels on Microblaze.

Can you give me an update on including this patch in
linux-2.6-microblaze? I realize the "const" was added as a result of a
checkpatch.pl complaint (see 6bd55f0bbaebb79b39e147aa864401fd0c94db82),
but it seems like a non-compiling kernel (even for trivial reasons) is
worse than a noisy Perl script.

thanks,
Graeme

2013-06-26 09:02:13

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH] microblaze: Don't mark arch_kgdb_ops as const.

Hi Graeme,

On 06/23/2013 11:16 PM, Graeme Smecher wrote:
> Hi Michal,
>
> On 08/06/13 01:58 PM, Graeme Smecher wrote:
>> Hi all,
>>
>> On 08/06/13 09:52 AM, Graeme Smecher wrote:
>>> Other architectures don't do it, and it conflicts with the extern'd definition
>>> in include/linux/kgdb.h.
>>>
>>> Signed-off-by: Graeme Smecher<[email protected]>
>>> CC: Michal Simek<[email protected]>
>>> CC:[email protected]
>>> ---
>>> arch/microblaze/kernel/kgdb.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/microblaze/kernel/kgdb.c b/arch/microblaze/kernel/kgdb.c
>>> index 8adc9244..09a5e82 100644
>>> --- a/arch/microblaze/kernel/kgdb.c
>>> +++ b/arch/microblaze/kernel/kgdb.c
>>> @@ -141,7 +141,7 @@ void kgdb_arch_exit(void)
>>> /*
>>> * Global data
>>> */
>>> -const struct kgdb_arch arch_kgdb_ops = {
>>> +struct kgdb_arch arch_kgdb_ops = {
>>> #ifdef __MICROBLAZEEL__
>>> .gdb_bpt_instr = {0x18, 0x00, 0x0c, 0xba}, /* brki r16, 0x18 */
>>> #else
>>
>> It's worth noting that this patch is flagged by checkpatch:
>>
>> WARNING: struct kgdb_arch should normally be const
>> #25: FILE: arch/microblaze/kernel/kgdb.c:144:
>> +struct kgdb_arch arch_kgdb_ops = {
>>
>> total: 0 errors, 1 warnings, 8 lines checked
>>
>>
>> However, without it, my kernel fails to compile:
>>
>> CC arch/microblaze/kernel/kgdb.o
>> arch/microblaze/kernel/kgdb.c:144:24: error: conflicting type
>> qualifiers for 'arch_kgdb_ops'
>> include/linux/kgdb.h:284:26: note: previous declaration of
>> 'arch_kgdb_ops' was here
>> make[1]: *** [arch/microblaze/kernel/kgdb.o] Error 1
>> make: *** [arch/microblaze/kernel] Error 2
>>
>>
>> The checkpatch test was a response to this (rejected) patch series:
>>
>> http://lkml.indiana.edu/hypermail/linux/kernel/0912.1/02659.html
>>
>> I'm the lucky victim, here, since I'm using gcc 4.6.4 (which notices the conflicting qualifiers), while other Microblaze users are still working with gcc 4.1.2 (which didn't care.) All other architectures (including mn10300, arc, and hexagon, which were accepted after the checkpatch commit) ignore the checkpatch rule and declare arch_kgdb_ops non-const.
>>
>> In any case, I wanted to point out (a) that the patch fails checkpatch, (b) that I'm submitting it anyway, and (c) that I'm utterly ambivalent about how the struct is declared, provided I can compile working kernels on Microblaze.
>
> Can you give me an update on including this patch in linux-2.6-microblaze? I realize the "const" was added as a result of a checkpatch.pl complaint (see 6bd55f0bbaebb79b39e147aa864401fd0c94db82), but it seems like a non-compiling kernel (even for trivial reasons) is worse than a noisy Perl script.

Sorry for delay.
Yes, you are right. I will extend description to connect it with that cleanup patch
and will also create follow up to enable kgdb in defconfig to be sure that it is tested regularly.

Applied.

Thanks,
Michal



--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



Attachments:
signature.asc (263.00 B)
OpenPGP digital signature